Reject invalid object types for incoming transactions.

TYPE_PTR and TYPE_FDA should only be used for scatter-gather
transactions, which is not supported by libbinder. Because userspace is
responsible for cleaning up the transaction objects, we can't just
reject the transaction outright; instead, free the objects and make sure
the Parcel won't contain them going forward.

While we're at it, also reject weak binders now, since those are no
longer supported.

For Parcels which were expecting valid objects, those will now fail to
unparcel, and return an error to the caller, which seems reasonable.
For Parcels which didn't expect any objects, the Parcel will be read
out successfully, but also no harm should be done, as there's no way to
propagate the bad objects.

Bug: 135930648
Test: atest binderLibTest
Change-Id: I2a4b408d6dcf1a67f3093d40061cb6159a0444f9

2 files changed, 51 insertions, 0 deletions

diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index ed6c834af8..3a7a7a9c3f 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -2354,6 +2354,22 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize,
             mObjectsSize = 0;
             break;
         }
+        const flat_binder_object* flat
+            = reinterpret_cast<const flat_binder_object*>(mData + offset);
+        uint32_t type = flat->hdr.type;
+        if (!(type == BINDER_TYPE_BINDER || type == BINDER_TYPE_HANDLE ||
+              type == BINDER_TYPE_FD)) {
+            // We should never receive other types (eg BINDER_TYPE_FDA) as long as we don't support
+            // them in libbinder. If we do receive them, it probably means a kernel bug; try to
+            // recover gracefully by clearing out the objects, and releasing the objects we do
+            // know about.
+            android_errorWriteLog(0x534e4554, "135930648");
+            ALOGE("%s: unsupported type object (%" PRIu32 ") at offset %" PRIu64 "\n",
+                  __func__, type, (uint64_t)offset);
+            releaseObjects();
+            mObjectsSize = 0;
+            break;
+        }
         minOffset = offset + sizeof(flat_binder_object);
     }
     scanForFds();
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 495b2f9d9a..5c6cf9db64 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -72,6 +72,7 @@ enum BinderLibTestTranscationCode {
     BINDER_LIB_TEST_CREATE_BINDER_TRANSACTION,
     BINDER_LIB_TEST_GET_WORK_SOURCE_TRANSACTION,
     BINDER_LIB_TEST_ECHO_VECTOR,
+    BINDER_LIB_TEST_REJECT_BUF,
 };
 
 pid_t start_server_process(int arg2, bool usePoll = false)
@@ -1024,6 +1025,34 @@ TEST_F(BinderLibTest, VectorSent) {
     EXPECT_EQ(readValue, testValue);
 }
 
+TEST_F(BinderLibTest, BufRejected) {
+    Parcel data, reply;
+    uint32_t buf;
+    sp<IBinder> server = addServer();
+    ASSERT_TRUE(server != nullptr);
+
+    binder_buffer_object obj {
+        .hdr = { .type = BINDER_TYPE_PTR },
+        .buffer = reinterpret_cast<binder_uintptr_t>((void*)&buf),
+        .length = 4,
+        .flags = 0,
+    };
+    data.setDataCapacity(1024);
+    // Write a bogus object at offset 0 to get an entry in the offset table
+    data.writeFileDescriptor(0);
+    EXPECT_EQ(data.objectsCount(), 1);
+    uint8_t *parcelData = const_cast<uint8_t*>(data.data());
+    // And now, overwrite it with the buffer object
+    memcpy(parcelData, &obj, sizeof(obj));
+    data.setDataSize(sizeof(obj));
+
+    status_t ret = server->transact(BINDER_LIB_TEST_REJECT_BUF, data, &reply);
+    // Either the kernel should reject this transaction (if it's correct), but
+    // if it's not, the server implementation should return an error if it
+    // finds an object in the received Parcel.
+    EXPECT_NE(NO_ERROR, ret);
+}
+
 class BinderLibTestService : public BBinder
 {
     public:
@@ -1306,6 +1335,9 @@ class BinderLibTestService : public BBinder
                 reply->writeUint64Vector(vector);
                 return NO_ERROR;
             }
+            case BINDER_LIB_TEST_REJECT_BUF: {
+                return data.objectsCount() == 0 ? BAD_VALUE : NO_ERROR;
+            }
             default:
                 return UNKNOWN_TRANSACTION;
             };
@@ -1337,6 +1369,9 @@ int run_server(int index, int readypipefd, bool usePoll)
          */
         testService->setExtension(new BBinder());
 
+        // Required for test "BufRejected'
+        testService->setRequestingSid(true);
+
         /*
          * We need this below, but can't hold a sp<> because it prevents the
          * node from being cleaned up automatically. It's safe in this case