commit 45e2e95c2ffeb2d978e2cce80b729ef6ada3b8d2
author Fyodor Kupolov <fkupolov@google.com> Mon Feb 13 18:35:12 2017 -0800
committer Fyodor Kupolov <fkupolov@google.com> Thu Feb 16 09:50:00 2017 -0800
tree 992c98155bec887d70df51eb7b90a35a1fe564a9
parent 3d52f79be4dba94f046709fabc1bfb911a670709

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

libs/androidfw/CursorWindow.cpp

diff --git a/libs/androidfw/CursorWindow.cpp b/libs/androidfw/CursorWindow.cpp
index 166863c..5694115 100644
--- a/libs/androidfw/CursorWindow.cpp
+++ b/libs/androidfw/CursorWindow.cpp
@@ -98,9 +98,14 @@
             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*/);

libs/androidfw/include/androidfw/CursorWindow.h

diff --git a/libs/androidfw/include/androidfw/CursorWindow.h b/libs/androidfw/include/androidfw/CursorWindow.h
index f543565..ad64b24 100644
--- a/libs/androidfw/include/androidfw/CursorWindow.h
+++ b/libs/androidfw/include/androidfw/CursorWindow.h
@@ -17,6 +17,7 @@
 #ifndef _ANDROID__DATABASE_WINDOW_H
 #define _ANDROID__DATABASE_WINDOW_H
 
+#include <inttypes.h>
 #include <stddef.h>
 #include <stdint.h>
 
@@ -128,12 +129,13 @@
     inline const char* getFieldSlotValueString(FieldSlot* fieldSlot,
             size_t* outSizeIncludingNull) {
         *outSizeIncludingNull = fieldSlot->data.buffer.size;
-        return static_cast<char*>(offsetToPtr(fieldSlot->data.buffer.offset));
+        return static_cast<char*>(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 +168,16 @@
     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 %" PRIu32 " out of bounds, max value %zu", offset, mSize);
+            return NULL;
+        }
+        if (offset + bufferSize > mSize) {
+            ALOGE("End offset %" PRIu32 " out of bounds, max value %zu",
+                    offset + bufferSize, mSize);
+            return NULL;
+        }
         return static_cast<uint8_t*>(mData) + offset;
     }