Merge remote-tracking branch 'mpg123/master' into master-with-github-ci

NEWS

@@ -1,8 +1,12 @@
 1.32.8
 ------
-- libmpg123: Add sections to assembly to support PAC/BTI code
-  for aarch64 (-mbranch-protection variants), thanks to Bill Roberts
-  (github PR 15).
+- libmpg123:
+-- Add sections to assembly to support PAC/BTI code
+   for aarch64 (-mbranch-protection variants), thanks to Bill Roberts
+   (github PR 15).
+-- Prevent premature application of header info into decoding structure,
+   at worst having triggered out-of-bounds writes of decoded PCM data
+   (bug 322, again).
 
 1.32.7
 ------

src/libmpg123/frame.c

@@ -511,6 +511,7 @@ static void frame_fixed_reset(mpg123_handle *fr)
 {
 	frame_icy_reset(fr);
 	INT123_open_bad(fr);
+	memset(&(fr->hdr), 0, sizeof(fr->hdr));
 	fr->to_decode = FALSE;
 	fr->to_ignore = FALSE;
 	fr->metaflags = 0;
@@ -524,15 +525,12 @@ static void frame_fixed_reset(mpg123_handle *fr)
 	fr->clip = 0;
 	fr->oldhead = 0;
 	fr->firsthead = 0;
-	fr->lay = 0;
 	fr->vbr = MPG123_CBR;
 	fr->abr_rate = 0;
 	fr->track_frames = 0;
 	fr->track_samples = -1;
-	fr->framesize=0; 
 	fr->mean_frames = 0;
 	fr->mean_framesize = 0;
-	fr->freesize = 0;
 	fr->lastscale = -1;
 	fr->rva.level[0] = -1;
 	fr->rva.level[1] = -1;
@@ -567,8 +565,7 @@ static void frame_fixed_reset(mpg123_handle *fr)
 	fr->icy.next = 0;
 #endif
 	fr->halfphase = 0; /* here or indeed only on first-time init? */
-	fr->error_protection = 0;
-	fr->freeformat_framesize = fr->p.freeformat_framesize;
+	fr->hdr.freeformat_framesize = fr->p.freeformat_framesize;
 	fr->enc_delay = -1;
 	fr->enc_padding = -1;
 	memset(fr->id3buf, 0, sizeof(fr->id3buf));
@@ -627,7 +624,7 @@ int attribute_align_arg mpg123_framedata(mpg123_handle *mh, unsigned long *heade
 
 	if(header    != NULL) *header    = mh->oldhead;
 	if(bodydata  != NULL) *bodydata  = mh->bsbuf;
-	if(bodybytes != NULL) *bodybytes = mh->framesize;
+	if(bodybytes != NULL) *bodybytes = mh->hdr.framesize;
 
 	return MPG123_OK;
 }
@@ -900,9 +897,9 @@ static int64_t ignoreframe(mpg123_handle *fr)
 {
 	int64_t preshift = fr->p.preframes;
 	/* Layer 3 _really_ needs at least one frame before. */
-	if(fr->lay==3 && preshift < 1) preshift = 1;
+	if(fr->hdr.lay==3 && preshift < 1) preshift = 1;
 	/* Layer 1 & 2 reall do not need more than 2. */
-	if(fr->lay!=3 && preshift > 2) preshift = 2;
+	if(fr->hdr.lay!=3 && preshift > 2) preshift = 2;
 
 	return fr->firstframe - preshift;
 }
@@ -949,7 +946,7 @@ void INT123_frame_set_frameseek(mpg123_handle *fr, int64_t fe)
 void INT123_frame_skip(mpg123_handle *fr)
 {
 #ifndef NO_LAYER3
-	if(fr->lay == 3) INT123_set_pointer(fr, 1, 512);
+	if(fr->hdr.lay == 3) INT123_set_pointer(fr, 1, 512);
 #endif
 }
 

src/libmpg123/frame.h

@@ -96,6 +96,33 @@ enum frame_state_flags
 	,FRAME_DECODER_LIVE  = 0x8  /**<     1000 Decoder can be used. */
 };
 
+// separate frame header structure for safe decoding of headers without
+// modifying the main frame struct before we are sure that we can read a
+// frame into it
+struct frame_header
+{
+	int lay;
+	// lots of flags that could share storage, should reform that
+	int lsf; /* 0: MPEG 1.0; 1: MPEG 2.0/2.5 -- both used as bool and array index! */
+	int mpeg25;
+	int error_protection;
+	int bitrate_index;
+	int sampling_frequency;
+	int padding;
+	int extension;
+	int mode;
+	int mode_ext;
+	int copyright;
+	int original;
+	int emphasis;
+	// Even 16 bit int is enough for MAXFRAMESIZE
+	int framesize; /* computed framesize */
+	int freeformat;
+	int freeformat_framesize;
+	// Derived from header and checked against the above.
+	int ssize;
+};
+
 /* There is a lot to condense here... many ints can be merged as flags; though the main space is still consumed by buffers. */
 struct mpg123_handle_struct
 {
@@ -197,26 +224,12 @@ struct mpg123_handle_struct
 	int single;
 	int II_sblimit;
 	int down_sample_sblimit;
-	int lsf; /* 0: MPEG 1.0; 1: MPEG 2.0/2.5 -- both used as bool and array index! */
 	/* Many flags in disguise as integers... wasting bytes. */
-	int mpeg25;
 	int down_sample;
 	int header_change;
-	int lay;
+	struct frame_header hdr;
 	long spf; /* cached count of samples per frame */
 	int (*do_layer)(mpg123_handle *);
-	int error_protection;
-	int bitrate_index;
-	int sampling_frequency;
-	int padding;
-	int extension;
-	int mode;
-	int mode_ext;
-	int copyright;
-	int original;
-	int emphasis;
-	int framesize; /* computed framesize */
-	int freesize;  /* free format frame size */
 	enum mpg123_vbr vbr; /* 1 if variable bitrate was detected */
 	int64_t num; /* frame offset ... */
 	int64_t input_offset; /* byte offset of this frame in input stream */
@@ -225,8 +238,6 @@ struct mpg123_handle_struct
 	int state_flags;
 	char silent_resync; /* Do not complain for the next n resyncs. */
 	unsigned char* xing_toc; /* The seek TOC from Xing header. */
-	int freeformat;
-	long freeformat_framesize;
 
 	/* bitstream info; bsi */
 	int bitindex;
@@ -253,7 +264,6 @@ struct mpg123_handle_struct
 	double mean_framesize;
 	int64_t mean_frames;
 	int fsizeold;
-	int ssize;
 	unsigned int bitreservoir;
 	unsigned char bsspace[2][MAXFRAMESIZE+512+4]; /* MAXFRAMESIZE */
 	unsigned char *bsbuf;

src/libmpg123/layer1.c

@@ -217,7 +217,7 @@ int INT123_do_layer1(mpg123_handle *fr)
 	real (*fraction)[SBLIMIT] = fr->layer1.fraction; /* fraction[2][SBLIMIT] */
 	int single = fr->single;
 
-	fr->jsbound = (fr->mode == MPG_MD_JOINT_STEREO) ? (fr->mode_ext<<2)+4 : 32;
+	fr->jsbound = (fr->hdr.mode == MPG_MD_JOINT_STEREO) ? (fr->hdr.mode_ext<<2)+4 : 32;
 
 	if(stereo == 1 || single == SINGLE_MIX) /* I don't see mixing handled here */
 	single = SINGLE_LEFT;

src/libmpg123/layer2.c

@@ -313,10 +313,10 @@ static void II_select_table(mpg123_handle *fr)
 	const struct al_table *tables[5] = { alloc_0, alloc_1, alloc_2, alloc_3 , alloc_4 };
 	const int sblims[5] = { 27 , 30 , 8, 12 , 30 };
 
-	if(fr->sampling_frequency >= 3)	/* Or equivalent: (fr->lsf == 1) */
+	if(fr->hdr.sampling_frequency >= 3)	/* Or equivalent: (fr->lsf == 1) */
 	table = 4;
 	else
-	table = translate[fr->sampling_frequency][2-fr->stereo][fr->bitrate_index];
+	table = translate[fr->hdr.sampling_frequency][2-fr->stereo][fr->hdr.bitrate_index];
 
 	sblim = sblims[table];
 	fr->alloc      = tables[table];
@@ -337,7 +337,7 @@ int INT123_do_layer2(mpg123_handle *fr)
 	int single = fr->single;
 
 	II_select_table(fr);
-	fr->jsbound = (fr->mode == MPG_MD_JOINT_STEREO) ? (fr->mode_ext<<2)+4 : fr->II_sblimit;
+	fr->jsbound = (fr->hdr.mode == MPG_MD_JOINT_STEREO) ? (fr->hdr.mode_ext<<2)+4 : fr->II_sblimit;
 
 	if(fr->jsbound > fr->II_sblimit)
 	{

src/libmpg123/layer3.c

@@ -135,16 +135,16 @@ static int III_get_side_info(mpg123_handle *fr, struct III_sideinfo *si,int ster
 	int powdiff = (single == SINGLE_MIX) ? 4 : 0;
 
 	const int tabs[2][5] = { { 2,9,5,3,4 } , { 1,8,1,2,9 } };
-	const int *tab = tabs[fr->lsf];
+	const int *tab = tabs[fr->hdr.lsf];
 
 	{ /* First ensure we got enough bits available. */
 		unsigned int needbits = 0;
 		needbits += tab[1]; /* main_data_begin */
 		needbits += stereo == 1 ? tab[2] : tab[3]; /* private */
-		if(!fr->lsf)
+		if(!fr->hdr.lsf)
 			needbits += stereo*4; /* scfsi */
 		/* For each granule for each channel ... */
-		needbits += tab[0]*stereo*(29+tab[4]+1+22+(!fr->lsf?1:0)+2);
+		needbits += tab[0]*stereo*(29+tab[4]+1+22+(!fr->hdr.lsf?1:0)+2);
 		if(fr->bits_avail < needbits) \
 		{
 			if(NOQUIET)
@@ -162,7 +162,7 @@ static int III_get_side_info(mpg123_handle *fr, struct III_sideinfo *si,int ster
 
 		/*  overwrite main_data_begin for the really available bit reservoir */
 		backbits(fr, tab[1]);
-		if(fr->lsf == 0)
+		if(fr->hdr.lsf == 0)
 		{
 			fr->wordpointer[0] = (unsigned char) (fr->bitreservoir >> 1);
 			fr->wordpointer[1] = (unsigned char) ((fr->bitreservoir & 1) << 7);
@@ -171,19 +171,19 @@ static int III_get_side_info(mpg123_handle *fr, struct III_sideinfo *si,int ster
 
 		/* zero "side-info" data for a silence-frame
 		without touching audio data used as bit reservoir for following frame */
-		memset(fr->wordpointer+2, 0, fr->ssize-2);
+		memset(fr->wordpointer+2, 0, fr->hdr.ssize-2);
 
 		/* reread the new bit reservoir offset */
 		si->main_data_begin = getbits(fr, tab[1]);
 	}
 
 	/* Keep track of the available data bytes for the bit reservoir.
 	   CRC is included in ssize already. */
-	fr->bitreservoir = fr->bitreservoir + fr->framesize - fr->ssize;
+	fr->bitreservoir = fr->bitreservoir + fr->hdr.framesize - fr->hdr.ssize;
 
 	/* Limit the reservoir to the max for MPEG 1.0 or 2.x . */
-	if(fr->bitreservoir > (unsigned int) (fr->lsf == 0 ? 511 : 255))
-	fr->bitreservoir = (fr->lsf == 0 ? 511 : 255);
+	if(fr->bitreservoir > (unsigned int) (fr->hdr.lsf == 0 ? 511 : 255))
+	fr->bitreservoir = (fr->hdr.lsf == 0 ? 511 : 255);
 
 	/* Now back into less commented territory. It's code. It works. */
 
@@ -192,7 +192,7 @@ static int III_get_side_info(mpg123_handle *fr, struct III_sideinfo *si,int ster
 	else 
 	si->private_bits = getbits(fr, tab[3]);
 
-	if(!fr->lsf) for(ch=0; ch<stereo; ch++)
+	if(!fr->hdr.lsf) for(ch=0; ch<stereo; ch++)
 	{
 		si->ch[ch].gr[0].scfsi = -1;
 		si->ch[ch].gr[1].scfsi = getbits(fr, 4);
@@ -257,14 +257,14 @@ static int III_get_side_info(mpg123_handle *fr, struct III_sideinfo *si,int ster
 			}
 
 			/* region_count/start parameters are implicit in this case. */       
-			if( (!fr->lsf || (gr_info->block_type == 2)) && !fr->mpeg25)
+			if( (!fr->hdr.lsf || (gr_info->block_type == 2)) && !fr->hdr.mpeg25)
 			{
 				gr_info->region1start = 36>>1;
 				gr_info->region2start = 576>>1;
 			}
 			else
 			{
-				if(fr->mpeg25)
+				if(fr->hdr.mpeg25)
 				{ 
 					int r0c,r1c;
 					if((gr_info->block_type == 2) && (!gr_info->mixed_block_flag) ) r0c = 5;
@@ -299,7 +299,7 @@ static int III_get_side_info(mpg123_handle *fr, struct III_sideinfo *si,int ster
 			gr_info->block_type = 0;
 			gr_info->mixed_block_flag = 0;
 		}
-		if(!fr->lsf) gr_info->preflag = get1bit(fr);
+		if(!fr->hdr.lsf) gr_info->preflag = get1bit(fr);
 
 		gr_info->scalefac_scale = get1bit(fr);
 		gr_info->count1table_select = get1bit(fr);
@@ -1824,7 +1824,7 @@ int INT123_do_layer3(mpg123_handle *fr)
 	int stereo = fr->stereo;
 	int single = fr->single;
 	int ms_stereo,i_stereo;
-	int sfreq = fr->sampling_frequency;
+	int sfreq = fr->hdr.sampling_frequency;
 	int stereo1,granules;
 
 	if(stereo == 1)
@@ -1837,14 +1837,14 @@ int INT123_do_layer3(mpg123_handle *fr)
 	else
 	stereo1 = 2;
 
-	if(fr->mode == MPG_MD_JOINT_STEREO)
+	if(fr->hdr.mode == MPG_MD_JOINT_STEREO)
 	{
-		ms_stereo = (fr->mode_ext & 0x2)>>1;
-		i_stereo  = fr->mode_ext & 0x1;
+		ms_stereo = (fr->hdr.mode_ext & 0x2)>>1;
+		i_stereo  = fr->hdr.mode_ext & 0x1;
 	}
 	else ms_stereo = i_stereo = 0;
 
-	granules = fr->lsf ? 1 : 2;
+	granules = fr->hdr.lsf ? 1 : 2;
 
 	/* quick hack to keep the music playing */
 	/* after having seen this nasty test file... */
@@ -1859,7 +1859,7 @@ int INT123_do_layer3(mpg123_handle *fr)
 	if(fr->pinfo)
 	{
 		fr->pinfo->maindata = sideinfo.main_data_begin;
-		fr->pinfo->padding  = fr->padding;
+		fr->pinfo->padding  = fr->hdr.padding;
 	}
 #endif
 	for(gr=0;gr<granules;gr++)
@@ -1880,7 +1880,7 @@ int INT123_do_layer3(mpg123_handle *fr)
 					,	gr_info->part2_3_length, fr->bits_avail );
 				return clip;
 			}
-			if(fr->lsf)
+			if(fr->hdr.lsf)
 			part2bits = III_get_scale_factors_2(fr, scalefacs[0],gr_info,0);
 			else
 			part2bits = III_get_scale_factors_1(fr, scalefacs[0],gr_info,0,gr);
@@ -1920,7 +1920,7 @@ int INT123_do_layer3(mpg123_handle *fr)
 		{
 			struct gr_info_s *gr_info = &(sideinfo.ch[1].gr[gr]);
 			long part2bits;
-			if(fr->lsf) 
+			if(fr->hdr.lsf) 
 			part2bits = III_get_scale_factors_2(fr, scalefacs[1],gr_info,i_stereo);
 			else
 			part2bits = III_get_scale_factors_1(fr, scalefacs[1],gr_info,1,gr);
@@ -1970,7 +1970,7 @@ int INT123_do_layer3(mpg123_handle *fr)
 				}
 			}
 
-			if(i_stereo) III_i_stereo(hybridIn,scalefacs[1],gr_info,sfreq,ms_stereo,fr->lsf);
+			if(i_stereo) III_i_stereo(hybridIn,scalefacs[1],gr_info,sfreq,ms_stereo,fr->hdr.lsf);
 
 			if(ms_stereo || i_stereo || (single == SINGLE_MIX) )
 			{

src/libmpg123/libmpg123.c

@@ -457,7 +457,7 @@ int attribute_align_arg mpg123_getstate(mpg123_handle *mh, enum mpg123_state key
 			theval = mh->enc_padding;
 		break;
 		case MPG123_DEC_DELAY:
-			theval = mh->lay == 3 ? GAPLESS_DELAY : -1;
+			theval = mh->hdr.lay == 3 ? GAPLESS_DELAY : -1;
 		break;
 		default:
 			mh->err = MPG123_BAD_KEY;
@@ -1241,25 +1241,25 @@ static int init_track(mpg123_handle *mh)
 	b = init_track(mh); \
 	if(b < 0) return b; \
  \
-	mi->version = mh->mpeg25 ? MPG123_2_5 : (mh->lsf ? MPG123_2_0 : MPG123_1_0); \
-	mi->layer = mh->lay; \
+	mi->version = mh->hdr.mpeg25 ? MPG123_2_5 : (mh->hdr.lsf ? MPG123_2_0 : MPG123_1_0); \
+	mi->layer = mh->hdr.lay; \
 	mi->rate = INT123_frame_freq(mh); \
-	switch(mh->mode) \
+	switch(mh->hdr.mode) \
 	{ \
 		case 0: mi->mode = MPG123_M_STEREO; break; \
 		case 1: mi->mode = MPG123_M_JOINT;  break; \
 		case 2: mi->mode = MPG123_M_DUAL;   break; \
 		case 3: mi->mode = MPG123_M_MONO;   break; \
 		default: mi->mode = 0; /* Nothing good to do here. */ \
 	} \
-	mi->mode_ext = mh->mode_ext; \
-	mi->framesize = mh->framesize+4; /* Include header. */ \
+	mi->mode_ext = mh->hdr.mode_ext; \
+	mi->framesize = mh->hdr.framesize+4; /* Include header. */ \
 	mi->flags = 0; \
-	if(mh->error_protection) mi->flags |= MPG123_CRC; \
-	if(mh->copyright)        mi->flags |= MPG123_COPYRIGHT; \
-	if(mh->extension)        mi->flags |= MPG123_PRIVATE; \
-	if(mh->original)         mi->flags |= MPG123_ORIGINAL; \
-	mi->emphasis = mh->emphasis; \
+	if(mh->hdr.error_protection) mi->flags |= MPG123_CRC; \
+	if(mh->hdr.copyright)        mi->flags |= MPG123_COPYRIGHT; \
+	if(mh->hdr.extension)        mi->flags |= MPG123_PRIVATE; \
+	if(mh->hdr.original)         mi->flags |= MPG123_ORIGINAL; \
+	mi->emphasis = mh->hdr.emphasis; \
 	mi->bitrate  = INT123_frame_bitrate(mh); \
 	mi->abr_rate = mh->abr_rate; \
 	mi->vbr = mh->vbr; \