Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Leap:15.1:ARM
mp3gain
mp3gain-1.6.1-libmpg123_fixup.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File mp3gain-1.6.1-libmpg123_fixup.patch of Package mp3gain
From: Thomas Orgis <thomas.orgis@gmx.de> Date: 2018-01-04 13:48:11 +0100 ------------------------------------------------------------------------ r2 | thomas | 2018-02-04 10:48:22 +0100 (dom 04 de feb de 2018) | 16 líneas Clean up the libmpg123 usage, also fixing crash by reproducer for CVE-2017-14407. This adds checks for return values to the mpg123 API calls, those were missing in my first dirty patch. Also, the output format (channel count) and the number of decoded samples are checked after each decoded frame now, to ensure that we do not deviate from mp3gain's view. A test for this the reproducer file for CVE-2017-14407, available at https://github.com/asarubbo/poc/blob/master/00345-aacgain-stackoverflow-filterYule . The CVE itself is older than the libmpg123 usage, but the bad file is nevertheless a nice example of mp3gain and mpg123 parsers deviating: Mp3gain code things that the channel count changed, while libmpg123 rejected that change from broken frame headers. Buffer overflow results (observable with CFLAGS=-fsanitize=address LDFLAGS=-lasan). Gapless decoding is deactivated, figuring that we want the raw samples from the file without offset games. There can be small changes of computed gain from that, but usually the padding should be short bits of silence anyway, nothing too fancy. Index: mp3gain.c =================================================================== --- mp3gain.c (revisión: 1) +++ mp3gain.c (revisión: 2) @@ -2126,9 +2126,19 @@ #ifdef AACGAIN if (!aacH) #endif - mh = mpg123_new(NULL, NULL); - mpg123_param(mh, MPG123_ADD_FLAGS, MPG123_FORCE_FLOAT, 0.); - mpg123_open_feed(mh); + /* We want mpg123 decoding to float only, and without fancy sample cutting, */ + /* so that mp3gain and mpg123 agree on sample offsets. The padding should */ + /* have no big effect on gain anyway. */ + if( + !(mh = mpg123_new(NULL, &decodeSuccess)) || + MPG123_OK != mpg123_param(mh, MPG123_ADD_FLAGS, MPG123_FORCE_FLOAT, 0.) || + MPG123_OK != mpg123_param(mh, MPG123_REMOVE_FLAGS, MPG123_GAPLESS, 0.) || + MPG123_OK != mpg123_open_feed(mh) + ) { + fprintf( stderr, "Failed to create/setup mpg123 handle:\n", + mh ? mpg123_strerror(mh) : mpg123_plain_strerror(decodeSuccess) ); + exit(1); + } if (tagInfo[mainloop].recalc == 0) { maxsample = tagInfo[mainloop].trackPeak * 32767.0; maxgain = tagInfo[mainloop].maxGain; @@ -2319,20 +2329,43 @@ unsigned char *decout; /* Get gain values directly, not from decoder. */ scanFrameGain(); - mpg123_feed(mh, curframe, bytesinframe); + /* Feed the frames to libmpg123. As they are pre-parsed already, */ + /* we tolerate no deviations. Calls for more data mean that the */ + /* mpg123 parser disagrees with us here, which is fatal. */ + /* It could also mean free format streams where you need to seek */ + /* ahead. But: mp3gain will not work on those anyway. A rework */ + /* might just rely on libmpg123's parser and pull the raw data */ + /* from there. */ + if(MPG123_OK != mpg123_feed(mh, curframe, bytesinframe)) { + fprintf(stderr, "Feeding mpg123 failed:\n", mpg123_strerror(mh)); + exit(1); + } + /* We will check output format for each frame, as there is the */ + /* case where mp3gain may think the channel count changed, but */ + /* mpg123 does not (CVE-2017-14407). */ decodeSuccess = mpg123_decode_frame(mh, NULL, &decout, &decbytes); if(decodeSuccess == MPG123_NEW_FORMAT) - { + decodeSuccess = mpg123_decode_frame(mh, NULL, &decout, &decbytes); + if(decodeSuccess == MPG123_OK) { int enc = 0, channels = 0; mpg123_getformat(mh, NULL, &channels, &enc); + /* Could also check that sampling freq matches ... */ if(enc != MPG123_ENC_FLOAT_32 || channels != nchan) { fprintf(stderr, "Unexpected format returned by libmpg123.\n"); exit(1); } - decodeSuccess = mpg123_decode_frame(mh, NULL, &decout, &decbytes); + } else { + fprintf(stderr, "Failed to decode MPEG frame: %s\n", mpg123_strerror(mh)); + exit(1); } nprocsamp = decbytes / sizeof(float) / nchan; + /* Paranoia triggered by CVE-2017-14407. */ + if(nprocsamp > sizeof(lsamples)/sizeof(Float_t)) + { + fprintf(stderr, "Too many samples in libmpg123 output.\n"); + exit(1); + } convert_decout((float*)decout, nprocsamp, nchan, lsamples, rsamples); maxsample = find_maxsample((float*)decout, nprocsamp*nchan, maxsample); #ifdef WIN32 ------------------------------------------------------------------------ r3 | thomas | 2018-02-04 10:52:33 +0100 (dom 04 de feb de 2018) | 1 línea Add missing %s to libmpg123 error printouts. Index: mp3gain.c =================================================================== --- mp3gain.c (revisión: 2) +++ mp3gain.c (revisión: 3) @@ -2135,7 +2135,7 @@ MPG123_OK != mpg123_param(mh, MPG123_REMOVE_FLAGS, MPG123_GAPLESS, 0.) || MPG123_OK != mpg123_open_feed(mh) ) { - fprintf( stderr, "Failed to create/setup mpg123 handle:\n", + fprintf( stderr, "Failed to create/setup mpg123 handle: %s\n", mh ? mpg123_strerror(mh) : mpg123_plain_strerror(decodeSuccess) ); exit(1); } @@ -2337,7 +2337,7 @@ /* might just rely on libmpg123's parser and pull the raw data */ /* from there. */ if(MPG123_OK != mpg123_feed(mh, curframe, bytesinframe)) { - fprintf(stderr, "Feeding mpg123 failed:\n", mpg123_strerror(mh)); + fprintf(stderr, "Feeding mpg123 failed: %s\n", mpg123_strerror(mh)); exit(1); } /* We will check output format for each frame, as there is the */ ------------------------------------------------------------------------ r7 | thomas | 2018-02-04 13:25:16 +0100 (dom 04 de feb de 2018) | 14 líneas Fix libmpg123 usage with respect to MPG123_NEED_MORE. Current libmpg123 always wants to check if there is a valid header after the first frame and thus always returns MPG123_NEED_MORE after feeding one and only one frame of MPEG data. Since API version 45 (mpg123 1.26.0, to be released), this can be avoided. This change makes mp3gain work both with new and old libmpg123, only giving a one-line note about the fact with the latter. The computation will miss the last frame with old libmpg123, but the effect for the overall gain is small, if measurable at all. The disabling of gapless decoding helps a bit. Index: mp3gain.c =================================================================== --- mp3gain.c (revisión: 6) +++ mp3gain.c (revisión: 7) @@ -2129,11 +2129,16 @@ #endif /* We want mpg123 decoding to float only, and without fancy sample cutting, */ /* so that mp3gain and mpg123 agree on sample offsets. The padding should */ - /* have no big effect on gain anyway. */ + /* have no big effect on gain anyway. Seek buffer needs to be disabled to */ + /* avoid MPG123_NEED_MORE after feeding the first frame. */ if( !(mh = mpg123_new(NULL, &decodeSuccess)) || MPG123_OK != mpg123_param(mh, MPG123_ADD_FLAGS, MPG123_FORCE_FLOAT, 0.) || - MPG123_OK != mpg123_param(mh, MPG123_REMOVE_FLAGS, MPG123_GAPLESS, 0.) || + MPG123_OK != mpg123_param(mh, MPG123_REMOVE_FLAGS, MPG123_GAPLESS|MPG123_SEEKBUFFER, 0.) || + MPG123_OK != mpg123_param(mh, MPG123_VERBOSE, 10, 0.) || +#if (MPG123_API_VERSION >= 45) + MPG123_OK != mpg123_param(mh, MPG123_ADD_FLAGS, MPG123_NO_READAHEAD, 0.) || +#endif MPG123_OK != mpg123_open_feed(mh) ) { fprintf( stderr, "Failed to create/setup mpg123 handle: %s\n", @@ -2357,8 +2362,18 @@ exit(1); } } else { +#if (MPG123_API_VERSION < 45) + if(decodeSuccess == MPG123_NEED_MORE) + { + fprintf(stderr, "Delaying a frame in decoding with old libmpg123.\n"); + decbytes = 0; + } else { +#endif fprintf(stderr, "Failed to decode MPEG frame: %s\n", mpg123_strerror(mh)); exit(1); +#if (MPG123_API_VERSION < 45) + } +#endif } nprocsamp = decbytes / sizeof(float) / nchan; /* Paranoia triggered by CVE-2017-14407. */ ------------------------------------------------------------------------
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
