Debian patchset for CVE-2017-68{29..38} and two other vulnerabilities: https://salsa.debian.org/multimedia-team/audiofile/commit/242f019#a064ca928f514268d4bae308e2e3990138341b76: * Address several vulnerabilities (Closes: #857651) - Always check the number of coefficients (CVE-2017-6827 CVE-2017-6828 CVE-2017-6832 CVE-2017-6833 CVE-2017-6835 CVE-2017-6837) - clamp index values to fix index overflow in IMA.cpp (CVE-2017-6829) - Check for multiplication overflow in sfconvert (CVE-2017-6830 CVE-2017-6834 CVE-2017-6836 CVE-2017-6838) - Actually fail when error occurs in parseFormat (CVE-2017-6831) - Check for multiplication overflow in MSADPCM decodeSample (CVE-2017-6839) * Fix signature of multiplyCheckOverflow. It returns a bool, not an int * Check for division by zero in BlockCodec::runPull From a2e9eab8ea87c4ffc494d839ebb4ea145eb9f2e6 Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Mon, 6 Mar 2017 18:59:26 +0100 Subject: [PATCH] Actually fail when error occurs in parseFormat When there's an unsupported number of bits per sample or an invalid number of samples per block, don't only print an error message using the error handler, but actually stop parsing the file. This fixes #35 (also reported at https://bugzilla.opensuse.org/show_bug.cgi?id=1026983 and https://blogs.gentoo.org/ago/2017/02/20/audiofile-heap-based-buffer-overflow-in-imadecodeblockwave-ima-cpp/ ) --- libaudiofile/WAVE.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libaudiofile/WAVE.cpp b/libaudiofile/WAVE.cpp index 0e81cf7..d762249 100644 --- a/libaudiofile/WAVE.cpp +++ b/libaudiofile/WAVE.cpp @@ -326,6 +326,7 @@ status WAVEFile::parseFormat(const Tag &id, uint32_t size) { _af_error(AF_BAD_NOT_IMPLEMENTED, "IMA ADPCM compression supports only 4 bits per sample"); + return AF_FAIL; } int bytesPerBlock = (samplesPerBlock + 14) / 8 * 4 * channelCount; @@ -333,6 +334,7 @@ status WAVEFile::parseFormat(const Tag &id, uint32_t size) { _af_error(AF_BAD_CODEC_CONFIG, "Invalid samples per block for IMA ADPCM compression"); + return AF_FAIL; } track->f.sampleWidth = 16; -- 2.11.0 From c48e4c6503f7dabd41f11d4c9c7b7f8960e7f2c0 Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Mon, 6 Mar 2017 12:51:22 +0100 Subject: [PATCH] Always check the number of coefficients When building the library with NDEBUG, asserts are eliminated so it's better to always check that the number of coefficients is inside the array range. This fixes the 00191-audiofile-indexoob issue in #41 --- libaudiofile/WAVE.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libaudiofile/WAVE.cpp b/libaudiofile/WAVE.cpp index 0e81cf7..61f9541 100644 --- a/libaudiofile/WAVE.cpp +++ b/libaudiofile/WAVE.cpp @@ -281,6 +281,12 @@ status WAVEFile::parseFormat(const Tag &id, uint32_t size) /* numCoefficients should be at least 7. */ assert(numCoefficients >= 7 && numCoefficients <= 255); + if (numCoefficients < 7 || numCoefficients > 255) + { + _af_error(AF_BAD_HEADER, + "Bad number of coefficients"); + return AF_FAIL; + } m_msadpcmNumCoefficients = numCoefficients; -- 2.11.0 From: Antonio Larrosa Date: Thu, 9 Mar 2017 10:21:18 +0100 Subject: Check for division by zero in BlockCodec::runPull --- libaudiofile/modules/BlockCodec.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libaudiofile/modules/BlockCodec.cpp b/libaudiofile/modules/BlockCodec.cpp index 4731be1..eb2fb4d 100644 --- a/libaudiofile/modules/BlockCodec.cpp +++ b/libaudiofile/modules/BlockCodec.cpp @@ -47,7 +47,7 @@ void BlockCodec::runPull() // Read the compressed data. ssize_t bytesRead = read(m_inChunk->buffer, m_bytesPerPacket * blockCount); - int blocksRead = bytesRead >= 0 ? bytesRead / m_bytesPerPacket : 0; + int blocksRead = (bytesRead >= 0 && m_bytesPerPacket > 0) ? bytesRead / m_bytesPerPacket : 0; // Decompress into m_outChunk. for (int i=0; i Date: Mon, 6 Mar 2017 13:43:53 +0100 Subject: [PATCH] Check for multiplication overflow in MSADPCM decodeSample Check for multiplication overflow (using __builtin_mul_overflow if available) in MSADPCM.cpp decodeSample and return an empty decoded block if an error occurs. This fixes the 00193-audiofile-signintoverflow-MSADPCM case of #41 --- libaudiofile/modules/BlockCodec.cpp | 5 ++-- libaudiofile/modules/MSADPCM.cpp | 47 +++++++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/libaudiofile/modules/BlockCodec.cpp b/libaudiofile/modules/BlockCodec.cpp index 45925e8..4731be1 100644 --- a/libaudiofile/modules/BlockCodec.cpp +++ b/libaudiofile/modules/BlockCodec.cpp @@ -52,8 +52,9 @@ void BlockCodec::runPull() // Decompress into m_outChunk. for (int i=0; i(m_inChunk->buffer) + i * m_bytesPerPacket, - static_cast(m_outChunk->buffer) + i * m_framesPerPacket * m_track->f.channelCount); + if (decodeBlock(static_cast(m_inChunk->buffer) + i * m_bytesPerPacket, + static_cast(m_outChunk->buffer) + i * m_framesPerPacket * m_track->f.channelCount)==0) + break; framesRead += m_framesPerPacket; } diff --git a/libaudiofile/modules/MSADPCM.cpp b/libaudiofile/modules/MSADPCM.cpp index 8ea3c85..ef9c38c 100644 --- a/libaudiofile/modules/MSADPCM.cpp +++ b/libaudiofile/modules/MSADPCM.cpp @@ -101,24 +101,60 @@ static const int16_t adaptationTable[] = 768, 614, 512, 409, 307, 230, 230, 230 }; +int firstBitSet(int x) +{ + int position=0; + while (x!=0) + { + x>>=1; + ++position; + } + return position; +} + +#ifndef __has_builtin +#define __has_builtin(x) 0 +#endif + +int multiplyCheckOverflow(int a, int b, int *result) +{ +#if (defined __GNUC__ && __GNUC__ >= 5) || ( __clang__ && __has_builtin(__builtin_mul_overflow)) + return __builtin_mul_overflow(a, b, result); +#else + if (firstBitSet(a)+firstBitSet(b)>31) // int is signed, so we can't use 32 bits + return true; + *result = a * b; + return false; +#endif +} + + // Compute a linear PCM value from the given differential coded value. static int16_t decodeSample(ms_adpcm_state &state, - uint8_t code, const int16_t *coefficient) + uint8_t code, const int16_t *coefficient, bool *ok=NULL) { int linearSample = (state.sample1 * coefficient[0] + state.sample2 * coefficient[1]) >> 8; + int delta; linearSample += ((code & 0x08) ? (code - 0x10) : code) * state.delta; linearSample = clamp(linearSample, MIN_INT16, MAX_INT16); - int delta = (state.delta * adaptationTable[code]) >> 8; + if (multiplyCheckOverflow(state.delta, adaptationTable[code], &delta)) + { + if (ok) *ok=false; + _af_error(AF_BAD_COMPRESSION, "Error decoding sample"); + return 0; + } + delta >>= 8; if (delta < 16) delta = 16; state.delta = delta; state.sample2 = state.sample1; state.sample1 = linearSample; + if (ok) *ok=true; return static_cast(linearSample); } @@ -212,13 +248,16 @@ int MSADPCM::decodeBlock(const uint8_t *encoded, int16_t *decoded) { uint8_t code; int16_t newSample; + bool ok; code = *encoded >> 4; - newSample = decodeSample(*state[0], code, coefficient[0]); + newSample = decodeSample(*state[0], code, coefficient[0], &ok); + if (!ok) return 0; *decoded++ = newSample; code = *encoded & 0x0f; - newSample = decodeSample(*state[1], code, coefficient[1]); + newSample = decodeSample(*state[1], code, coefficient[1], &ok); + if (!ok) return 0; *decoded++ = newSample; encoded++; -- 2.11.0 From 7d65f89defb092b63bcbc5d98349fb222ca73b3c Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Mon, 6 Mar 2017 13:54:52 +0100 Subject: [PATCH] Check for multiplication overflow in sfconvert Checks that a multiplication doesn't overflow when calculating the buffer size, and if it overflows, reduce the buffer size instead of failing. This fixes the 00192-audiofile-signintoverflow-sfconvert case in #41 --- sfcommands/sfconvert.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/sfcommands/sfconvert.c b/sfcommands/sfconvert.c index 80a1bc4..970a3e4 100644 --- a/sfcommands/sfconvert.c +++ b/sfcommands/sfconvert.c @@ -45,6 +45,33 @@ void printusage (void); void usageerror (void); bool copyaudiodata (AFfilehandle infile, AFfilehandle outfile, int trackid); +int firstBitSet(int x) +{ + int position=0; + while (x!=0) + { + x>>=1; + ++position; + } + return position; +} + +#ifndef __has_builtin +#define __has_builtin(x) 0 +#endif + +int multiplyCheckOverflow(int a, int b, int *result) +{ +#if (defined __GNUC__ && __GNUC__ >= 5) || ( __clang__ && __has_builtin(__builtin_mul_overflow)) + return __builtin_mul_overflow(a, b, result); +#else + if (firstBitSet(a)+firstBitSet(b)>31) // int is signed, so we can't use 32 bits + return true; + *result = a * b; + return false; +#endif +} + int main (int argc, char **argv) { if (argc == 2) @@ -323,8 +350,11 @@ bool copyaudiodata (AFfilehandle infile, AFfilehandle outfile, int trackid) { int frameSize = afGetVirtualFrameSize(infile, trackid, 1); - const int kBufferFrameCount = 65536; - void *buffer = malloc(kBufferFrameCount * frameSize); + int kBufferFrameCount = 65536; + int bufferSize; + while (multiplyCheckOverflow(kBufferFrameCount, frameSize, &bufferSize)) + kBufferFrameCount /= 2; + void *buffer = malloc(bufferSize); AFframecount totalFrames = afGetFrameCount(infile, AF_DEFAULT_TRACK); AFframecount totalFramesWritten = 0; -- 2.11.0 From 25eb00ce913452c2e614548d7df93070bf0d066f Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Mon, 6 Mar 2017 18:02:31 +0100 Subject: [PATCH] clamp index values to fix index overflow in IMA.cpp This fixes #33 (also reported at https://bugzilla.opensuse.org/show_bug.cgi?id=1026981 and https://blogs.gentoo.org/ago/2017/02/20/audiofile-global-buffer-overflow-in-decodesample-ima-cpp/) --- libaudiofile/modules/IMA.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libaudiofile/modules/IMA.cpp b/libaudiofile/modules/IMA.cpp index 7476d44..df4aad6 100644 --- a/libaudiofile/modules/IMA.cpp +++ b/libaudiofile/modules/IMA.cpp @@ -169,7 +169,7 @@ int IMA::decodeBlockWAVE(const uint8_t *encoded, int16_t *decoded) if (encoded[1] & 0x80) m_adpcmState[c].previousValue -= 0x10000; - m_adpcmState[c].index = encoded[2]; + m_adpcmState[c].index = clamp(encoded[2], 0, 88); *decoded++ = m_adpcmState[c].previousValue; @@ -210,7 +210,7 @@ int IMA::decodeBlockQT(const uint8_t *encoded, int16_t *decoded) predictor -= 0x10000; state.previousValue = clamp(predictor, MIN_INT16, MAX_INT16); - state.index = encoded[1] & 0x7f; + state.index = clamp(encoded[1] & 0x7f, 0, 88); encoded += 2; for (int n=0; n Date: Fri, 10 Mar 2017 15:40:02 +0100 Subject: [PATCH] Fix signature of multiplyCheckOverflow. It returns a bool, not an int --- libaudiofile/modules/MSADPCM.cpp | 2 +- sfcommands/sfconvert.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libaudiofile/modules/MSADPCM.cpp b/libaudiofile/modules/MSADPCM.cpp index ef9c38c..d8c9553 100644 --- a/libaudiofile/modules/MSADPCM.cpp +++ b/libaudiofile/modules/MSADPCM.cpp @@ -116,7 +116,7 @@ int firstBitSet(int x) #define __has_builtin(x) 0 #endif -int multiplyCheckOverflow(int a, int b, int *result) +bool multiplyCheckOverflow(int a, int b, int *result) { #if (defined __GNUC__ && __GNUC__ >= 5) || ( __clang__ && __has_builtin(__builtin_mul_overflow)) return __builtin_mul_overflow(a, b, result); diff --git a/sfcommands/sfconvert.c b/sfcommands/sfconvert.c index 970a3e4..367f7a5 100644 --- a/sfcommands/sfconvert.c +++ b/sfcommands/sfconvert.c @@ -60,7 +60,7 @@ int firstBitSet(int x) #define __has_builtin(x) 0 #endif -int multiplyCheckOverflow(int a, int b, int *result) +bool multiplyCheckOverflow(int a, int b, int *result) { #if (defined __GNUC__ && __GNUC__ >= 5) || ( __clang__ && __has_builtin(__builtin_mul_overflow)) return __builtin_mul_overflow(a, b, result); -- 2.11.0