format_mp3: Fix a possible crash in mp3_read().

This patch fixes a potential crash in mp3_read() by not assuming that
dbuf has enough data to finish filling up the output buffer.  The patch
also makes sure that the dbuf state gets reset after we know we read
everything out of it already.

In passing, this patch includes some other cleanups of this module,
including stripping trailing whitespace, formatting fixes based on
coding guidelines, and removing a number of unused members from the
private state struct.

(closes issue ASTERISK-19761)
Reported by: Chris Maciejewsk
Tested by: Chris Maciejewsk


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@366296 65c4cc65-6c06-0410-ace0-fbb531ad65f3

addons/format_mp3.c

@@ -48,20 +48,20 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #define MP3_DCACHE 8192
 
 struct mp3_private {
-	char waste[AST_FRIENDLY_OFFSET];	/* Buffer for sending frames, etc */
+	/*! state for the mp3 decoder */
-	char empty;				/* Empty character */
-	int lasttimeout;
-	int maxlen;
-	struct timeval last;
 	struct mpstr mp;
+	/*! buffer to hold mp3 data after read from disk */
 	char sbuf[MP3_SCACHE];
+	/*! buffer for slinear audio after being decoded out of sbuf */
 	char dbuf[MP3_DCACHE];
+	/*! how much data has been written to the output buffer in the ast_filestream */
 	int buflen;
+	/*! how much data has been written to sbuf */
 	int sbuflen;
+	/*! how much data is left to be read out of dbuf, starting at dbufoffset */
 	int dbuflen;
+	/*! current offset for reading data out of dbuf */
 	int dbufoffset;
-	int sbufoffset;
-	int lastseek;
 	int offset;
 	long seek;
 };
@@ -117,7 +117,7 @@ static int mp3_squeue(struct ast_filestream *s)
 	struct mp3_private *p = s->_private;
 	int res=0;
 
-	p->lastseek = ftell(s->f);
+	res = ftell(s->f);
 	p->sbuflen = fread(p->sbuf, 1, MP3_SCACHE, s->f);
 	if(p->sbuflen < 0) {
 		ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", p->sbuflen, strerror(errno));
@@ -194,36 +194,41 @@ static struct ast_frame *mp3_read(struct ast_filestream *s, int *whennext)
 	int delay =0;
 	int save=0;
 
-	/* Send a frame from the file to the appropriate channel */
+	/* Pre-populate the buffer that holds audio to be returned (dbuf) */
-
+	if (mp3_queue(s)) {
-	if(mp3_queue(s))
 		return NULL;
+	}
 
-	if(p->dbuflen) {
+	if (p->dbuflen) {
-		for(p->buflen=0; p->buflen < MP3_BUFLEN && p->buflen < p->dbuflen; p->buflen++) {
+		/* Read out what's waiting in dbuf */
-			s->buf[p->buflen + AST_FRIENDLY_OFFSET] = p->dbuf[p->buflen+p->dbufoffset];
+		for (p->buflen = 0; p->buflen < MP3_BUFLEN && p->buflen < p->dbuflen; p->buflen++) {
-			p->sbufoffset++;
+			s->buf[p->buflen + AST_FRIENDLY_OFFSET] = p->dbuf[p->buflen + p->dbufoffset];
 		}
 		p->dbufoffset += p->buflen;
 		p->dbuflen -= p->buflen;
+	}
 
-		if(p->buflen < MP3_BUFLEN) {
+	if (p->buflen < MP3_BUFLEN) {
-			if(mp3_queue(s))
+		/* dbuf didn't have enough, so reset dbuf, fill it back up and continue */
-				return NULL;
+		p->dbuflen = p->dbufoffset = 0;
 
-			for(save = p->buflen; p->buflen < MP3_BUFLEN; p->buflen++) {
+		if (mp3_queue(s)) {
-				s->buf[p->buflen + AST_FRIENDLY_OFFSET] = p->dbuf[(p->buflen-save)+p->dbufoffset];
+			return NULL;
-				p->sbufoffset++;
+		}
+
+		/* Make sure dbuf has enough to complete this read attempt */
+		if (p->dbuflen >= (MP3_BUFLEN - p->buflen)) {
+			for (save = p->buflen; p->buflen < MP3_BUFLEN; p->buflen++) {
+				s->buf[p->buflen + AST_FRIENDLY_OFFSET] = p->dbuf[(p->buflen - save) + p->dbufoffset];
 			}
 			p->dbufoffset += (MP3_BUFLEN - save);
 			p->dbuflen -= (MP3_BUFLEN - save);
-
 		}
 
 	}
 
 	p->offset += p->buflen;
-	delay = p->buflen/2;
+	delay = p->buflen / 2;
 	s->fr.frametype = AST_FRAME_VOICE;
 	s->fr.subclass.codec = AST_FORMAT_SLINEAR;
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, p->buflen);