Merge "spdif: fix possible buffer overflow" into qt-dev am: 31759bef66 am: 4e58bb55b1 am: c7eea4d86b

Original change: https://googleplex-android-review.googlesource.com/c/platform/system/media/+/11445945

Change-Id: I021e40197bc40631c3f88a89c4dbf6b48eedc584

4 files changed, 31 insertions, 11 deletions

diff --git a/audio_utils/include/audio_utils/spdif/SPDIFEncoder.h b/audio_utils/include/audio_utils/spdif/SPDIFEncoder.h
index 3c84d73c..1a432d82 100644
--- a/audio_utils/include/audio_utils/spdif/SPDIFEncoder.h
+++ b/audio_utils/include/audio_utils/spdif/SPDIFEncoder.h
@@ -83,6 +83,7 @@ public:
 
 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 e640c743..53094c5e 100644
--- a/audio_utils/spdif/AC3FrameScanner.cpp
+++ b/audio_utils/spdif/AC3FrameScanner.cpp
@@ -194,7 +194,13 @@ bool AC3FrameScanner::parseHeader()
 
         // 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 81de943b..893dfca4 100644
--- a/audio_utils/spdif/FrameScanner.cpp
+++ b/audio_utils/spdif/FrameScanner.cpp
@@ -36,7 +36,7 @@ FrameScanner::FrameScanner(int dataType,
  , 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 594438ca..4a8a02a3 100644
--- a/audio_utils/spdif/SPDIFEncoder.cpp
+++ b/audio_utils/spdif/SPDIFEncoder.cpp
@@ -103,14 +103,21 @@ int SPDIFEncoder::getBytesPerOutputFrame()
     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;
     }
@@ -128,14 +135,13 @@ void SPDIFEncoder::writeBurstBufferShorts(const uint16_t *buffer, size_t numShor
 // 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;
@@ -221,9 +227,16 @@ void SPDIFEncoder::startDataBurst()
 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.