Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 82c7531e authored by Martijn Coenen's avatar Martijn Coenen
Browse files

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
parent d0391001

libs/binder/Parcel.cpp

+16 −0
Original line number Original line Diff line number Diff line
@@ -2354,6 +2354,22 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, 
            mObjectsSize = 0;

            mObjectsSize = 0;

            break;

            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);

        minOffset = offset + sizeof(flat_binder_object);

    }

    }

    scanForFds();

    scanForFds();

libs/binder/tests/binderLibTest.cpp

+35 −0
Original line number Original line Diff line number Diff line
@@ -72,6 +72,7 @@ enum BinderLibTestTranscationCode { 
    BINDER_LIB_TEST_CREATE_BINDER_TRANSACTION,

    BINDER_LIB_TEST_CREATE_BINDER_TRANSACTION,

    BINDER_LIB_TEST_GET_WORK_SOURCE_TRANSACTION,

    BINDER_LIB_TEST_GET_WORK_SOURCE_TRANSACTION,

    BINDER_LIB_TEST_ECHO_VECTOR,

    BINDER_LIB_TEST_ECHO_VECTOR,

    BINDER_LIB_TEST_REJECT_BUF,

};

};





pid_t start_server_process(int arg2, bool usePoll = false)

pid_t start_server_process(int arg2, bool usePoll = false)
@@ -1024,6 +1025,34 @@ TEST_F(BinderLibTest, VectorSent) { 
    EXPECT_EQ(readValue, testValue);

    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

class BinderLibTestService : public BBinder

{

{

    public:

    public:
@@ -1306,6 +1335,9 @@ class BinderLibTestService : public BBinder 
                reply->writeUint64Vector(vector);

                reply->writeUint64Vector(vector);

                return NO_ERROR;

                return NO_ERROR;

            }

            }

            case BINDER_LIB_TEST_REJECT_BUF: {

                return data.objectsCount() == 0 ? BAD_VALUE : NO_ERROR;

            }

            default:

            default:

                return UNKNOWN_TRANSACTION;

                return UNKNOWN_TRANSACTION;

            };

            };
@@ -1337,6 +1369,9 @@ int run_server(int index, int readypipefd, bool usePoll) 
         */

         */

        testService->setExtension(new BBinder());

        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

         * 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

         * node from being cleaned up automatically. It's safe in this case