spdif: fix possible buffer overflow
SPDIF burst buffer could overflow if given bad data.
Check for frmsiz in header being too low.
Prevent numeric overflow in pending bytes calculation.
Prevent numeric overflow when checking for buffer overflow.
Bug: 145262423
Bug: 160265164
Test: tests/spdif_tests.cpp
Change-Id: Ifc7f8c8946388e19f39f35d649a5936568f8b9b2
Merged-In: Ifc7f8c8946388e19f39f35d649a5936568f8b9b2
(cherry picked from commit 1f73a728ef4b6d7d350c0715bdb10d59e5b5f9be)
diff --git a/audio_utils/include/audio_utils/spdif/SPDIFEncoder.h b/audio_utils/include/audio_utils/spdif/SPDIFEncoder.h
index 3c84d73..1a432d8 100644
--- a/audio_utils/include/audio_utils/spdif/SPDIFEncoder.h
+++ b/audio_utils/include/audio_utils/spdif/SPDIFEncoder.h
@@ -83,6 +83,7 @@
protected:
void clearBurstBuffer();
+ bool wouldOverflowBuffer(size_t numBytes) const; // Would this many bytes cause an overflow?
void writeBurstBufferShorts(const uint16_t* buffer, size_t numBytes);
void writeBurstBufferBytes(const uint8_t* buffer, size_t numBytes);
void sendZeroPad();
diff --git a/audio_utils/spdif/AC3FrameScanner.cpp b/audio_utils/spdif/AC3FrameScanner.cpp
index 4055bb0..b8418f6 100644
--- a/audio_utils/spdif/AC3FrameScanner.cpp
+++ b/audio_utils/spdif/AC3FrameScanner.cpp
@@ -194,7 +194,13 @@
// Frame size is explicit in EAC3. Paragraph E2.3.1.3
uint32_t frmsiz = ((mHeaderBuffer[2] & 0x07) << 8) + mHeaderBuffer[3];
- mFrameSizeBytes = (frmsiz + 1) * sizeof(int16_t);
+ uint32_t frameSizeBytes = (frmsiz + 1) * sizeof(int16_t);
+ if (frameSizeBytes < mHeaderLength) {
+ ALOGW("AC3 frame size = %d, less than header size = %d", frameSizeBytes, mHeaderLength);
+ android_errorWriteLog(0x534e4554, "145262423");
+ return false;
+ }
+ mFrameSizeBytes = frameSizeBytes;
uint32_t numblkscod = 3; // 6 blocks default
if (fscod == 3) {
diff --git a/audio_utils/spdif/FrameScanner.cpp b/audio_utils/spdif/FrameScanner.cpp
index 2b591e3..7c37eb6 100644
--- a/audio_utils/spdif/FrameScanner.cpp
+++ b/audio_utils/spdif/FrameScanner.cpp
@@ -36,7 +36,7 @@
, mFormatDumpCount(0)
, mSampleRate(0)
, mRateMultiplier(1)
- , mFrameSizeBytes(0)
+ , mFrameSizeBytes(headerLength) // minimum
, mDataType(dataType)
, mDataTypeInfo(0)
{
diff --git a/audio_utils/spdif/SPDIFEncoder.cpp b/audio_utils/spdif/SPDIFEncoder.cpp
index 5c62299..b918e26 100644
--- a/audio_utils/spdif/SPDIFEncoder.cpp
+++ b/audio_utils/spdif/SPDIFEncoder.cpp
@@ -101,14 +101,21 @@
return SPDIF_ENCODED_CHANNEL_COUNT * sizeof(int16_t);
}
+bool SPDIFEncoder::wouldOverflowBuffer(size_t numBytes) const {
+ // Avoid numeric overflow when calculating whether the buffer would overflow.
+ return (numBytes > mBurstBufferSizeBytes)
+ || (mByteCursor > (mBurstBufferSizeBytes - numBytes)); // (max - n) won't overflow
+}
+
void SPDIFEncoder::writeBurstBufferShorts(const uint16_t *buffer, size_t numShorts)
{
// avoid static analyser warning
LOG_ALWAYS_FATAL_IF((mBurstBuffer == NULL), "mBurstBuffer never allocated");
+
mByteCursor = (mByteCursor + 1) & ~1; // round up to even byte
size_t bytesToWrite = numShorts * sizeof(uint16_t);
- if ((mByteCursor + bytesToWrite) > mBurstBufferSizeBytes) {
- ALOGE("SPDIFEncoder: Burst buffer overflow!");
+ if (wouldOverflowBuffer(bytesToWrite)) {
+ ALOGE("SPDIFEncoder::%s() Burst buffer overflow!", __func__);
reset();
return;
}
@@ -126,14 +133,13 @@
// Big and Little Endian CPUs.
void SPDIFEncoder::writeBurstBufferBytes(const uint8_t *buffer, size_t numBytes)
{
- size_t bytesToWrite = numBytes;
- if ((mByteCursor + bytesToWrite) > mBurstBufferSizeBytes) {
- ALOGE("SPDIFEncoder: Burst buffer overflow!");
+ if (wouldOverflowBuffer(numBytes)) {
+ ALOGE("SPDIFEncoder::%s() Burst buffer overflow!", __func__);
clearBurstBuffer();
return;
}
uint16_t pad = mBurstBuffer[mByteCursor >> 1];
- for (size_t i = 0; i < bytesToWrite; i++) {
+ for (size_t i = 0; i < numBytes; i++) {
if (mByteCursor & 1 ) {
pad |= *buffer++; // put second byte in LSB
mBurstBuffer[mByteCursor >> 1] = pad;
@@ -219,9 +225,16 @@
size_t SPDIFEncoder::startSyncFrame()
{
// Write start of encoded frame that was buffered in frame detector.
- size_t syncSize = mFramer->getHeaderSizeBytes();
- writeBurstBufferBytes(mFramer->getHeaderAddress(), syncSize);
- return mFramer->getFrameSizeBytes() - syncSize;
+ size_t headerSize = mFramer->getHeaderSizeBytes();
+ writeBurstBufferBytes(mFramer->getHeaderAddress(), headerSize);
+ // This is provided by the encoded audio file and may be invalid.
+ size_t frameSize = mFramer->getFrameSizeBytes();
+ if (frameSize < headerSize) {
+ ALOGE("SPDIFEncoder: invalid frameSize = %zu", frameSize);
+ return 0;
+ }
+ // Calculate how many more bytes we need to complete the frame.
+ return frameSize - headerSize;
}
// Wraps raw encoded data into a data burst.
