From 7b83d625aa360ea8d4e41584017e0d5179540c5b Mon Sep 17 00:00:00 2001 From: Fyodor Kupolov Date: Mon, 27 Feb 2017 17:33:18 -0800 Subject: [DO NOT MERGE] Check bounds in offsetToPtr Check whether specified offset belongs to mData. Also added a default argument bufferSize to check the end offset. Size of the ashmem descriptor can be modified between ashmem_get_size_region call and mmap. createFromParcel method was updated to check ashmem size again immediately after memory is mapped. Test: manual - using the test app from the bug Bug: 34128677 Change-Id: I3ecd1616a870ce20941ce9b20a1843d2b4295750 (cherry picked from commit 45e2e95c2ffeb2d978e2cce80b729ef6ada3b8d2) --- include/androidfw/CursorWindow.h | 16 +++++++++++++--- libs/androidfw/CursorWindow.cpp | 5 +++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/include/androidfw/CursorWindow.h b/include/androidfw/CursorWindow.h index 8a2979a3756d..250014e26426 100644 --- a/include/androidfw/CursorWindow.h +++ b/include/androidfw/CursorWindow.h @@ -128,12 +128,13 @@ public: inline const char* getFieldSlotValueString(FieldSlot* fieldSlot, size_t* outSizeIncludingNull) { *outSizeIncludingNull = fieldSlot->data.buffer.size; - return static_cast(offsetToPtr(fieldSlot->data.buffer.offset)); + return static_cast(offsetToPtr( + fieldSlot->data.buffer.offset, fieldSlot->data.buffer.size)); } inline const void* getFieldSlotValueBlob(FieldSlot* fieldSlot, size_t* outSize) { *outSize = fieldSlot->data.buffer.size; - return offsetToPtr(fieldSlot->data.buffer.offset); + return offsetToPtr(fieldSlot->data.buffer.offset, fieldSlot->data.buffer.size); } private: @@ -166,7 +167,16 @@ private: bool mReadOnly; Header* mHeader; - inline void* offsetToPtr(uint32_t offset) { + inline void* offsetToPtr(uint32_t offset, uint32_t bufferSize = 0) { + if (offset >= mSize) { + ALOGE("Offset %zu out of bounds, max value %zu", offset, mSize); + return NULL; + } + if (offset + bufferSize > mSize) { + ALOGE("End offset %zu out of bounds, max value %zu", + offset + bufferSize, mSize); + return NULL; + } return static_cast(mData) + offset; } diff --git a/libs/androidfw/CursorWindow.cpp b/libs/androidfw/CursorWindow.cpp index 0f54edb6f915..a54410d7d0e3 100644 --- a/libs/androidfw/CursorWindow.cpp +++ b/libs/androidfw/CursorWindow.cpp @@ -98,9 +98,14 @@ status_t CursorWindow::createFromParcel(Parcel* parcel, CursorWindow** outCursor if (dupAshmemFd < 0) { result = -errno; } else { + // the size of the ashmem descriptor can be modified between ashmem_get_size_region + // call and mmap, so we'll check again immediately after memory is mapped void* data = ::mmap(NULL, size, PROT_READ, MAP_SHARED, dupAshmemFd, 0); if (data == MAP_FAILED) { result = -errno; + } else if (ashmem_get_size_region(dupAshmemFd) != size) { + ::munmap(data, size); + result = BAD_VALUE; } else { CursorWindow* window = new CursorWindow(name, dupAshmemFd, data, size, true /*readOnly*/); -- cgit v1.2.3-59-g8ed1b