Control: tags -1 + patch

On Sun, Jul 02, 2017 at 11:12:36AM +0200, Salvatore Bonaccorso wrote:
> Source: mpg123
> Version: 1.25.0-1
> Severity: important
> Tags: upstream security
> 
> Hi,
> 
> the following vulnerability was published for mpg123.
> 
> CVE-2017-10683[0]:
> | In mpg123 1.25.0, there is a heap-based buffer over-read in the
> | convert_latin1 function in libmpg123/id3.c. A crafted input will lead
> | to a remote denial of service attack.
> 
> This was reported at [1], but Hanno Boeck recently reported [2] as
> well.
> 
> Looking at both cases i think those should be the same issues, and
> upstream has a patch for the issue.
> 
> If you fix the vulnerability please also make sure to include the
> CVE (Common Vulnerabilities & Exposures) id in your changelog entry.
> 
> For further information see:
> 
> [0] https://security-tracker.debian.org/tracker/CVE-2017-10683
>     https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-10683
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1465819
> [2] https://sourceforge.net/p/mpg123/bugs/252/

Attaching the extracted patch.

Regards,
Salvatore
Index: NEWS
===================================================================
--- NEWS	(revision 4251)
+++ NEWS	(revision 4252)
@@ -2,6 +2,9 @@
 -------
 - libmpg123:
 -- Avoid memset(NULL, 0, 0) to calm down the paranoid.
+-- Fix bug 252, invalid read of size 1 in ID3v2 parser due to forgotten
+   offset from the frame flag bytes (unnoticed in practice for a long
+   time).
 
 1.25.0: MP3 now patent-free worldwide!
 -------
Index: src/libmpg123/id3.c
===================================================================
--- src/libmpg123/id3.c	(revision 4251)
+++ src/libmpg123/id3.c	(revision 4252)
@@ -700,6 +700,8 @@
 
 	/* length-10 or length-20 (footer present); 4 synchsafe integers == 28 bit number  */
 	/* we have already read 10 bytes, so left are length or length+10 bytes belonging to tag */
+	/* Note: This is an 28 bit value in 32 bit storage, plenty of space for */
+	/* length+x for reasonable x. */
 	if(!synchsafe_to_long(buf+2,length))
 	{
 		if(NOQUIET) error4("Bad tag length (not synchsafe): 0x%02x%02x%02x%02x; You got a bad ID3 tag here.", buf[2],buf[3],buf[4],buf[5]);
@@ -764,13 +766,16 @@
 					char id[5];
 					unsigned long framesize;
 					unsigned long fflags; /* need 16 bits, actually */
+					/* bytes of frame title and of framesize value */
+					int head_part = fr->id3v2.version > 2 ? 4 : 3;
+					int flag_part = fr->id3v2.version > 2 ? 2 : 0;
 					id[4] = 0;
 					/* pos now advanced after ext head, now a frame has to follow */
-					while(tagpos < length-10) /* I want to read at least a full header */
+					/* I want to read at least one full header now. */
+					while(tagpos <= length-head_part-head_part-flag_part)
 					{
 						int i = 0;
 						unsigned long pos = tagpos;
-						int head_part = fr->id3v2.version == 2 ? 3 : 4; /* bytes of frame title and of framesize value */
 						/* level 1,2,3 - 0 is info from lame/info tag! */
 						/* rva tags with ascending significance, then general frames */
 						enum frame_types tt = unknown;
@@ -801,7 +806,7 @@
 							}
 							if(VERBOSE3) fprintf(stderr, "Note: ID3v2 %s frame of size %lu\n", id, framesize);
 							tagpos += head_part + framesize; /* the important advancement in whole tag */
-							if(tagpos > length)
+							if(tagpos > length-flag_part)
 							{
 								if(NOQUIET) error("Whoa! ID3v2 frame claims to be larger than the whole rest of the tag.");
 								break;
Index: .
===================================================================
--- .	(revision 4251)
+++ .	(revision 4252)

Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
## -0,0 +0,1 ##
   Merged /trunk:r4249
_______________________________________________
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Reply via email to