commit 694e9732a044d547862a890c030ee486d9cbd832
author Frederick Mayle <fmayle@google.com> Fri Jul 15 00:24:58 2022 +0000
committer Frederick Mayle <fmayle@google.com> Fri Jul 15 21:10:15 2022 +0000
tree c3b3d3ba14e4d0c7fa29c96b2d033d25a92c92b8
parent 53b6ffe5af3951e8784c451ef8c4ff19f3d6b196

libbinder: Validate the RPC object positions immediately

We already check the bounds when they are used, but it is easier to
reason about the code if they are validated before being stored.

The `freeData` call was redundant (handled by dtor).

Test: TH
Change-Id: I5160e72b5225b933068a3aa68eb5287bf7a29af1

libs/binder/Parcel.cpp

diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 72c1a1a..8b5d118 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -2590,6 +2590,22 @@
 
     LOG_ALWAYS_FATAL_IF(session == nullptr);
 
+    if (objectTableSize != ancillaryFds.size()) {
+        ALOGE("objectTableSize=%zu ancillaryFds.size=%zu", objectTableSize, ancillaryFds.size());
+        relFunc(data, dataSize, nullptr, 0);
+        return BAD_VALUE;
+    }
+    for (size_t i = 0; i < objectTableSize; i++) {
+        uint32_t minObjectEnd;
+        if (__builtin_add_overflow(objectTable[i], sizeof(RpcFields::ObjectType), &minObjectEnd) ||
+            minObjectEnd >= dataSize) {
+            ALOGE("received out of range object position: %" PRIu32 " (parcel size is %zu)",
+                  objectTable[i], dataSize);
+            relFunc(data, dataSize, nullptr, 0);
+            return BAD_VALUE;
+        }
+    }
+
     freeData();
     markForRpc(session);
 
@@ -2600,12 +2616,6 @@
     mDataSize = mDataCapacity = dataSize;
     mOwner = relFunc;
 
-    if (objectTableSize != ancillaryFds.size()) {
-        ALOGE("objectTableSize=%zu ancillaryFds.size=%zu", objectTableSize, ancillaryFds.size());
-        freeData(); // don't leak mData
-        return BAD_VALUE;
-    }
-
     rpcFields->mObjectPositions.reserve(objectTableSize);
     for (size_t i = 0; i < objectTableSize; i++) {
         rpcFields->mObjectPositions.push_back(objectTable[i]);