Fix buffer overflow with some id3 genres
authorHugo Villeneuve <hugo@hugovil.com>
Tue, 6 Sep 2011 04:33:47 +0000 (00:33 -0400)
committerHugo Villeneuve <hugo@hugovil.com>
Tue, 25 Mar 2014 03:27:04 +0000 (23:27 -0400)
A crash occurs (buffer overflow) with some id3 genre, for instance: "Chanson"
or "BritPop". The problem was due to the usage of sprintf function (instead of
snprintf) and an error of buffer size for this id3 field.

Origin: Frédéric Fauberteau ( triaxx ) - 2010-04-24 08:45:26 UTC
        http://sourceforge.net/tracker/?func=detail&aid=2991696&group_id=3714&atid=303714

src/id3.c

index 46c89bf..cd38528 100644 (file)
--- a/src/id3.c
+++ b/src/id3.c
@@ -28,6 +28,9 @@
 
 static void ID3Put(char *dest,char *src,int len,char *encoding);
 
+#define GENRE_MAX_DIGITS 6
+#define TRACK_MAX_DIGITS 3
+
 /* this array contains string representations of all known ID3 tags */
 /* taken from mp3id3 in the mp3tools 0.7 package */
 
@@ -252,8 +255,8 @@ gboolean ID3v2TagFile(char *filename, char *title, char *artist, char *album,
       
       if ( frames[ i ] ) {
        char *c_data = NULL;
-       char gen[ 5 ] = "(   )";
-       char trk[ 4 ] = "   ";
+       char gen[ GENRE_MAX_DIGITS ] = "(   )"; /* max unsigned char: 255 */
+       char trk[ TRACK_MAX_DIGITS ] = "  "; /* max CDDA tracks: 99 */
        
        switch( frameids[ i ] ) {
        case ID3FID_TITLE:
@@ -278,12 +281,12 @@ gboolean ID3v2TagFile(char *filename, char *title, char *artist, char *album,
          
        case ID3FID_CONTENTTYPE:
          c_data = gen;
-         sprintf( gen, "(%d)", genre ); /* XXX */
+         snprintf( gen, GENRE_MAX_DIGITS, "(%d)", genre );
          break;
          
        case ID3FID_TRACKNUM:
          c_data = trk;
-         sprintf( trk, "%d", tracknum ); /* XXX */
+         snprintf( trk, TRACK_MAX_DIGITS, "%d", tracknum );
          break;
          
        default: