Severe Remote Code Execution vulnerability

[lyuma]

The article at https://thehackernews.com/2022/04/critical-chipset-bug-opens-millions-of.html
reported on a severe security vulnerability in this repository, but I see no mention here about it.

The master branch has seen no updates in 6 years. Have the vulnerabilities been patched?

https://nvd.nist.gov/vuln/detail/CVE-2021-0674
https://nvd.nist.gov/vuln/detail/CVE-2021-0675
https://nvd.nist.gov/vuln/detail/CVE-2021-30351

[shoghicp]

After some short fuzzing sessions, they have definitely not been patched. The issues also run deeper and exist at many spots in the code (and additionally can cause double-free besides the out of bound read/writes).

Gave up trying to fully patch this walking colander for personal projects, frameLength too small causes double-free on deletion of the decoder (add your own bounds check here if desired, still exploitable in other sections).

Additionally, other places exist where bound checking is not done, and values coming from bitstream are fully trusted.

Do note this patch is incomplete for other additional general issues found.

diff --git a/codec/ALACBitUtilities.c b/codec/ALACBitUtilities.c
index 9414889..92a2c21 100644
--- a/codec/ALACBitUtilities.c
+++ b/codec/ALACBitUtilities.c
@@ -37,6 +37,8 @@ void BitBufferInit( BitBuffer * bits, uint8_t * buffer, uint32_t byteSize )
 	bits->byteSize	= byteSize;
 }
 
+#define AssertBufferReadBounds(b, n) RequireAction( b->end >= (b->cur + ((b->bitIndex + n) / 8 + ((b->bitIndex + n) % 8 > 0))), return 0; );
+
 // BitBufferRead
 //
 uint32_t BitBufferRead( BitBuffer * bits, uint8_t numBits )
@@ -45,6 +47,8 @@ uint32_t BitBufferRead( BitBuffer * bits, uint8_t numBits )
 	
 	//Assert( numBits <= 16 );
 
+	AssertBufferReadBounds(bits, numBits)
+
 	returnBits = ((uint32_t)bits->cur[0] << 16) | ((uint32_t)bits->cur[1] << 8) | ((uint32_t)bits->cur[2]);
 	returnBits = returnBits << bits->bitIndex;
 	returnBits &= 0x00FFFFFF;
@@ -55,7 +59,7 @@ uint32_t BitBufferRead( BitBuffer * bits, uint8_t numBits )
 	
 	bits->cur		+= (bits->bitIndex >> 3);
 	bits->bitIndex	&= 7;
-	
+
 	//Assert( bits->cur <= bits->end );
 	
 	return returnBits;
@@ -69,6 +73,8 @@ uint8_t BitBufferReadSmall( BitBuffer * bits, uint8_t numBits )
 	uint16_t		returnBits;
 	
 	//Assert( numBits <= 8 );
+
+	AssertBufferReadBounds(bits, numBits)
 	
 	returnBits = (bits->cur[0] << 8) | bits->cur[1];
 	returnBits = returnBits << bits->bitIndex;
@@ -92,6 +98,8 @@ uint8_t BitBufferReadOne( BitBuffer * bits )
 {
 	uint8_t		returnBits;
 
+	AssertBufferReadBounds(bits, 8)
+
 	returnBits = (bits->cur[0] >> (7 - bits->bitIndex)) & 1;
 
 	bits->bitIndex++;
diff --git a/codec/ALACDecoder.cpp b/codec/ALACDecoder.cpp
index ce3340d..c8f0d17 100644
--- a/codec/ALACDecoder.cpp
+++ b/codec/ALACDecoder.cpp
@@ -128,8 +128,13 @@ int32_t ALACDecoder::Init( void * inMagicCookie, uint32_t inMagicCookieSize )
         theConfig.sampleRate = Swap32BtoN(((ALACSpecificConfig *)theActualCookie)->sampleRate);
 
         mConfig = theConfig;
-        
+
+		//sanity checks
         RequireAction( mConfig.compatibleVersion <= kALACVersion, return kALAC_ParamError; );
+		RequireAction( mConfig.bitDepth == 16 || mConfig.bitDepth == 20 || mConfig.bitDepth == 24 || mConfig.bitDepth == 32, return kALAC_ParamError; );
+		RequireAction( mConfig.frameLength > 0 && mConfig.frameLength <= 16384, return kALAC_ParamError; );
+		RequireAction( mConfig.sampleRate > 0 && mConfig.sampleRate <= 384000, return kALAC_ParamError; );
+		RequireAction( mConfig.numChannels > 0 && mConfig.numChannels < kALACMaxChannels, return kALAC_ParamError; );
 
         // allocate mix buffers
         mMixBufferU = (int32_t *) calloc( mConfig.frameLength * sizeof(int32_t), 1 );
@@ -251,6 +256,8 @@ int32_t ALACDecoder::Decode( BitBuffer * bits, uint8_t * sampleBuffer, uint32_t
 				{
 					numSamples  = BitBufferRead( bits, 16 ) << 16;
 					numSamples |= BitBufferRead( bits, 16 );
+
+					RequireAction( numSamples <= mConfig.frameLength, status = kALAC_ParamError; goto Exit; );
 				}
 
 				if ( escapeFlag == 0 )
@@ -402,6 +409,8 @@ int32_t ALACDecoder::Decode( BitBuffer * bits, uint8_t * sampleBuffer, uint32_t
 				{
 					numSamples  = BitBufferRead( bits, 16 ) << 16;
 					numSamples |= BitBufferRead( bits, 16 );
+
+					RequireAction( numSamples <= mConfig.frameLength, status = kALAC_ParamError; goto Exit; );
 				}
 
 				if ( escapeFlag == 0 )

They're not patched.

I did a quick fuzz run ca. 20 hours ago and rediscovered CVE-2021-0674 CVE-2021-0675 almost instantly (few seconds)

In my case found out of bounds read in ALACEncoder (possibly CVE-2021-0674) and out of bounds write in DecodeALAC (possibly CVE-2021-0675)

[shoghicp]

They're not patched.

I did a quick fuzz run ca. 20 hours ago and rediscovered CVE-2021-0674 CVE-2021-0675 almost instantly (few seconds)

In my case found out of bounds read in ALACEncoder (possibly CVE-2021-0674) and out of bounds write in DecodeALAC (possibly CVE-2021-0675)

In my testing, if frame sizes etc. are small enough but everything else is fine enough, ALACDecoder destructor will trigger a double-free but haven't checked further, afaik it's a corrupted allocation by an OOB write.

Good to know. Maybe it will help others.

Below is the OOB read in ALACEncoder (wav to caf)

=================================================================
==222777==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x621000002500 at pc 0x0000004e7ffe bp 0x7fff1f779130 sp 0x7fff1f779128
READ of size 8 at 0x621000002500 thread T0
    #0 0x4e7ffd in ALACEncoder::EncodeMono(BitBuffer*, void*, unsigned int, unsigned int, unsigned int) /src/fuzzers/alacfuzzer/alac/codec/ALACEncoder.cpp:850:36
    #1 0x4e84fe in ALACEncoder::Encode(AudioFormatDescription, AudioFormatDescription, unsigned char*, unsigned char*, int*) /src/fuzzers/alacfuzzer/alac/codec/ALACEncoder.cpp:1056:18
    #2 0x4d63a0 in EncodeALAC(_IO_FILE*, _IO_FILE*, AudioFormatDescription, AudioFormatDescription, int) /src/fuzzers/alacfuzzer/alac/fuzzer/main.cpp:491:21
    #3 0x4d36cb in main /src/fuzzers/alacfuzzer/alac/fuzzer/main.cpp:160:13
    #4 0x7f4542deb0b2 in __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
    #5 0x429109 in _start (/home/user/alacfuzzer+0x429109)

Ahh ... oh so even the convert-utility has a bug. So I found this one, not the CVE-2021-0675

(caf to wav)

=================================================================
==226015==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x625000002108 at pc 0x000000440f9e bp 0x7ffcadcedd50 sp 0x7ffcadced518
WRITE of size 8336 at 0x625000002108 thread T0
    #0 0x440f9d in fread (/home/user/convert-utility+0x440f9d)
    #1 0x4d7a2b in DecodeALAC(_IO_FILE*, _IO_FILE*, AudioFormatDescription, AudioFormatDescription, int, unsigned int) /src/fuzzers/alacfuzzer/alac/convert-utility/main.cpp:660:73
    #2 0x4d3901 in main /src/fuzzers/alacfuzzer/alac/convert-utility/main.cpp:173:13
    #3 0x7f8a56dfb0b2 in __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
    #4 0x429109 in _start (/home/user/convert-utility+0x429109)

0x625000002108 is located 0 bytes to the right of 8200-byte region [0x625000000100,0x625000002108)