commit d8cc2062b6f02d06b5cc5eb2f0a157d5ea92249c
author Shruti Bihani <shrutibihani@google.com> Thu Jul 20 09:10:41 2023 +0000
committer Shruti Bihani <shrutibihani@google.com> Wed Jul 26 05:57:03 2023 +0000
tree ef17d53fe2a03153809a5ae9f0792a7229e19f9a
parent 02f8dca82bc89914c25fe4bd1450a7f83f6ea1f4

Fix heap buffer overflow issue flagged by fuzzer test.

OOB read occurs when the buffer index to be read is greater than the buffer size. Adding a check on buffer bounds fixes the issue.
Similar checks have been added wherever applicable on other such methods of the class.

Bug: 243400841
Test: Build mtp_data_packet_fuzzer and run on the target device

Change-Id: Ic3c0030d60a0e068aa5ddec6aa544bed5ba0ef8e

media/mtp/MtpDataPacket.cpp

diff --git a/media/mtp/MtpDataPacket.cpp b/media/mtp/MtpDataPacket.cpp
index 5dbcd08..6068d68 100644
--- a/media/mtp/MtpDataPacket.cpp
+++ b/media/mtp/MtpDataPacket.cpp
@@ -73,14 +73,14 @@
 }
 
 bool MtpDataPacket::getUInt8(uint8_t& value) {
-    if (mPacketSize - mOffset < sizeof(value))
+    if ((mPacketSize - mOffset < sizeof(value)) || (mOffset >= mBufferSize))
         return false;
     value = mBuffer[mOffset++];
     return true;
 }
 
 bool MtpDataPacket::getUInt16(uint16_t& value) {
-    if (mPacketSize - mOffset < sizeof(value))
+    if ((mPacketSize - mOffset < sizeof(value)) || ((mOffset+1) >= mBufferSize))
         return false;
     int offset = mOffset;
     value = (uint16_t)mBuffer[offset] | ((uint16_t)mBuffer[offset + 1] << 8);
@@ -89,7 +89,7 @@
 }
 
 bool MtpDataPacket::getUInt32(uint32_t& value) {
-    if (mPacketSize - mOffset < sizeof(value))
+    if ((mPacketSize - mOffset < sizeof(value)) || ((mOffset+3) >= mBufferSize))
         return false;
     int offset = mOffset;
     value = (uint32_t)mBuffer[offset] | ((uint32_t)mBuffer[offset + 1] << 8) |
@@ -99,7 +99,7 @@
 }
 
 bool MtpDataPacket::getUInt64(uint64_t& value) {
-    if (mPacketSize - mOffset < sizeof(value))
+    if ((mPacketSize - mOffset < sizeof(value)) || ((mOffset+7) >= mBufferSize))
         return false;
     int offset = mOffset;
     value = (uint64_t)mBuffer[offset] | ((uint64_t)mBuffer[offset + 1] << 8) |