Bug#508133: [pkg-mad-maintainers] Bug#508133: audacity: munmap_chunk(): invalid pointer: 0x00000000026f4eb0

[Kurt Roeckx]

tags 508133 + patch security
thanks

On Tue, Dec 09, 2008 at 06:59:08AM +0100, Max Kellermann wrote:
> 
> It's a raw PCM file (16 bit stereo, 44.1 or 48 kHz).  The crash is
> reproducible by invoking "audacity libmad-crash-test".

I've attached a diff that fixes it for me.  But I'm not really
happy with it.

I'm abusing the MAD_ERROR_LOSTSYNC which make it an existing
recoverable error.  I should probably create new errors instead.

I'm also not sure that the changes I've made in layer12.c also
don't affect layer3.c.  I just didn't see such problems in layer3.c
with your test file.


Kurt


--- libmad-0.15.1b.orig/layer12.c
+++ libmad-0.15.1b/layer12.c
@@ -134,6 +134,12 @@
   for (sb = 0; sb < bound; ++sb) {
     for (ch = 0; ch < nch; ++ch) {
       nb = mad_bit_read(&stream->ptr, 4);
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
 
       if (nb == 15) {
 	stream->error = MAD_ERROR_BADBITALLOC;
@@ -146,6 +152,12 @@
 
   for (sb = bound; sb < 32; ++sb) {
     nb = mad_bit_read(&stream->ptr, 4);
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
 
     if (nb == 15) {
       stream->error = MAD_ERROR_BADBITALLOC;
@@ -162,6 +174,12 @@
     for (ch = 0; ch < nch; ++ch) {
       if (allocation[ch][sb]) {
 	scalefactor[ch][sb] = mad_bit_read(&stream->ptr, 6);
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
 
 # if defined(OPT_STRICT)
 	/*
@@ -187,6 +205,12 @@
 	frame->sbsample[ch][s][sb] = nb ?
 	  mad_f_mul(I_sample(&stream->ptr, nb),
 		    sf_table[scalefactor[ch][sb]]) : 0;
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
       }
     }
 
@@ -195,6 +219,12 @@
 	mad_fixed_t sample;
 
 	sample = I_sample(&stream->ptr, nb);
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
 
 	for (ch = 0; ch < nch; ++ch) {
 	  frame->sbsample[ch][s][sb] =
@@ -403,7 +433,15 @@
     nbal = bitalloc_table[offsets[sb]].nbal;
 
     for (ch = 0; ch < nch; ++ch)
+    {
       allocation[ch][sb] = mad_bit_read(&stream->ptr, nbal);
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
+    }
   }
 
   for (sb = bound; sb < sblimit; ++sb) {
@@ -411,6 +449,13 @@
 
     allocation[0][sb] =
     allocation[1][sb] = mad_bit_read(&stream->ptr, nbal);
+
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
   }
 
   /* decode scalefactor selection info */
@@ -419,6 +464,12 @@
     for (ch = 0; ch < nch; ++ch) {
       if (allocation[ch][sb])
 	scfsi[ch][sb] = mad_bit_read(&stream->ptr, 2);
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
     }
   }
 
@@ -442,6 +493,12 @@
     for (ch = 0; ch < nch; ++ch) {
       if (allocation[ch][sb]) {
 	scalefactor[ch][sb][0] = mad_bit_read(&stream->ptr, 6);
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
 
 	switch (scfsi[ch][sb]) {
 	case 2:
@@ -452,11 +509,23 @@
 
 	case 0:
 	  scalefactor[ch][sb][1] = mad_bit_read(&stream->ptr, 6);
+		if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+		{
+			stream->error = MAD_ERROR_LOSTSYNC;
+			stream->sync = 0;
+			return -1;
+		}
 	  /* fall through */
 
 	case 1:
 	case 3:
 	  scalefactor[ch][sb][2] = mad_bit_read(&stream->ptr, 6);
+		if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+		{
+			stream->error = MAD_ERROR_LOSTSYNC;
+			stream->sync = 0;
+			return -1;
+		}
 	}
 
 	if (scfsi[ch][sb] & 1)
@@ -488,6 +557,12 @@
 	  index = offset_table[bitalloc_table[offsets[sb]].offset][index - 1];
 
 	  II_samples(&stream->ptr, &qc_table[index], samples);
+		if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+		{
+			stream->error = MAD_ERROR_LOSTSYNC;
+			stream->sync = 0;
+			return -1;
+		}
 
 	  for (s = 0; s < 3; ++s) {
 	    frame->sbsample[ch][3 * gr + s][sb] =
@@ -506,6 +581,12 @@
 	index = offset_table[bitalloc_table[offsets[sb]].offset][index - 1];
 
 	II_samples(&stream->ptr, &qc_table[index], samples);
+	if (mad_bit_nextbyte(&stream->ptr) > stream->next_frame)
+	{
+		stream->error = MAD_ERROR_LOSTSYNC;
+		stream->sync = 0;
+		return -1;
+	}
 
 	for (ch = 0; ch < nch; ++ch) {
 	  for (s = 0; s < 3; ++s) {
--- libmad-0.15.1b.orig/layer3.c
+++ libmad-0.15.1b/layer3.c
@@ -2608,6 +2608,12 @@
     next_md_begin = 0;
 
   md_len = si.main_data_begin + frame_space - next_md_begin;
+  if (md_len + MAD_BUFFER_GUARD > MAD_BUFFER_MDLEN)
+  {
+	stream->error = MAD_ERROR_LOSTSYNC;
+	stream->sync = 0;
+	return -1;
+  }
 
   frame_used = 0;