commit b3adc03435ac48f9b286d46b14752a2c43e8dc26
author Steven Moreland <smoreland@google.com> Fri May 14 20:41:11 2021 +0000
committer Steven Moreland <smoreland@google.com> Sat May 15 02:10:58 2021 +0000
tree 8216cc7f874e82eee24ce01c374ea96c2cad7e1c
parent d4f73fbf0f41d12dc744f3397f75a01e6dd0e516

libbinder_ndk: read size checks

Heuristic checks on maximum transaction sizes, like we have in C++. Some
OOM situations averted.

Bug: 131868573
Test: binder_parcel_fuzzer for a few minutes
Test: CtsNdkBinderTestCases
Change-Id: I45b400670012bb12cf0f517f189dd95cf087fa8b

libs/binder/ndk/parcel.cpp

diff --git a/libs/binder/ndk/parcel.cpp b/libs/binder/ndk/parcel.cpp
index ec7c7d8..b2f21c7 100644
--- a/libs/binder/ndk/parcel.cpp
+++ b/libs/binder/ndk/parcel.cpp
@@ -46,7 +46,8 @@
 template <typename T>
 using ArraySetter = void (*)(void* arrayData, size_t index, T value);
 
-binder_status_t WriteAndValidateArraySize(AParcel* parcel, bool isNullArray, int32_t length) {
+static binder_status_t WriteAndValidateArraySize(AParcel* parcel, bool isNullArray,
+                                                 int32_t length) {
     // only -1 can be used to represent a null array
     if (length < -1) return STATUS_BAD_VALUE;
 
@@ -61,12 +62,24 @@
 
     Parcel* rawParcel = parcel->get();
 
-    status_t status = rawParcel->writeInt32(static_cast<int32_t>(length));
+    status_t status = rawParcel->writeInt32(length);
     if (status != STATUS_OK) return PruneStatusT(status);
 
     return STATUS_OK;
 }
 
+static binder_status_t ReadAndValidateArraySize(const AParcel* parcel, int32_t* length) {
+    if (status_t status = parcel->get()->readInt32(length); status != STATUS_OK) {
+        return PruneStatusT(status);
+    }
+
+    if (*length < -1) return STATUS_BAD_VALUE;  // libbinder_ndk reserves these
+    if (*length <= 0) return STATUS_OK;         // null
+    if (static_cast<size_t>(*length) > parcel->get()->dataAvail()) return STATUS_NO_MEMORY;
+
+    return STATUS_OK;
+}
+
 template <typename T>
 binder_status_t WriteArray(AParcel* parcel, const T* array, int32_t length) {
     binder_status_t status = WriteAndValidateArraySize(parcel, array == nullptr, length);
@@ -111,10 +124,9 @@
     const Parcel* rawParcel = parcel->get();
 
     int32_t length;
-    status_t status = rawParcel->readInt32(&length);
-
-    if (status != STATUS_OK) return PruneStatusT(status);
-    if (length < -1) return STATUS_BAD_VALUE;
+    if (binder_status_t status = ReadAndValidateArraySize(parcel, &length); status != STATUS_OK) {
+        return status;
+    }
 
     T* array;
     if (!allocator(arrayData, length, &array)) return STATUS_NO_MEMORY;
@@ -140,10 +152,9 @@
     const Parcel* rawParcel = parcel->get();
 
     int32_t length;
-    status_t status = rawParcel->readInt32(&length);
-
-    if (status != STATUS_OK) return PruneStatusT(status);
-    if (length < -1) return STATUS_BAD_VALUE;
+    if (binder_status_t status = ReadAndValidateArraySize(parcel, &length); status != STATUS_OK) {
+        return status;
+    }
 
     char16_t* array;
     if (!allocator(arrayData, length, &array)) return STATUS_NO_MEMORY;
@@ -155,7 +166,7 @@
     if (__builtin_smul_overflow(sizeof(char16_t), length, &size)) return STATUS_NO_MEMORY;
 
     for (int32_t i = 0; i < length; i++) {
-        status = rawParcel->readChar(array + i);
+        status_t status = rawParcel->readChar(array + i);
 
         if (status != STATUS_OK) return PruneStatusT(status);
     }
@@ -189,10 +200,9 @@
     const Parcel* rawParcel = parcel->get();
 
     int32_t length;
-    status_t status = rawParcel->readInt32(&length);
-
-    if (status != STATUS_OK) return PruneStatusT(status);
-    if (length < -1) return STATUS_BAD_VALUE;
+    if (binder_status_t status = ReadAndValidateArraySize(parcel, &length); status != STATUS_OK) {
+        return status;
+    }
 
     if (!allocator(arrayData, length)) return STATUS_NO_MEMORY;
 
@@ -200,7 +210,7 @@
 
     for (int32_t i = 0; i < length; i++) {
         T readTarget;
-        status = (rawParcel->*read)(&readTarget);
+        status_t status = (rawParcel->*read)(&readTarget);
         if (status != STATUS_OK) return PruneStatusT(status);
 
         setter(arrayData, i, readTarget);
@@ -402,13 +412,10 @@
 binder_status_t AParcel_readStringArray(const AParcel* parcel, void* arrayData,
                                         AParcel_stringArrayAllocator allocator,
                                         AParcel_stringArrayElementAllocator elementAllocator) {
-    const Parcel* rawParcel = parcel->get();
-
     int32_t length;
-    status_t status = rawParcel->readInt32(&length);
-
-    if (status != STATUS_OK) return PruneStatusT(status);
-    if (length < -1) return STATUS_BAD_VALUE;
+    if (binder_status_t status = ReadAndValidateArraySize(parcel, &length); status != STATUS_OK) {
+        return status;
+    }
 
     if (!allocator(arrayData, length)) return STATUS_NO_MEMORY;
 
@@ -449,13 +456,10 @@
 binder_status_t AParcel_readParcelableArray(const AParcel* parcel, void* arrayData,
                                             AParcel_parcelableArrayAllocator allocator,
                                             AParcel_readParcelableElement elementReader) {
-    const Parcel* rawParcel = parcel->get();
-
     int32_t length;
-    status_t status = rawParcel->readInt32(&length);
-
-    if (status != STATUS_OK) return PruneStatusT(status);
-    if (length < -1) return STATUS_BAD_VALUE;
+    if (binder_status_t status = ReadAndValidateArraySize(parcel, &length); status != STATUS_OK) {
+        return status;
+    }
 
     if (!allocator(arrayData, length)) return STATUS_NO_MEMORY;