getpidcon ACL Bypass

Issues with getpidcon() usage is a long story in Android. The Jann Horn's issue report details that the hardware servicemanager was vulnerable and several similar bugs were already reported because of unsafe getpidcon() usage. This function is not a safe way to get the SELinux context of a calling process as explained by Jann Horn in one of his issues:

This is problematic because getpidcon($pid) is only safe to use if the caller either knows that the process originally referenced by $pid can't transition from zombie to dead (normally because it is the parent or ptracer of $pid) or if the caller can validate that the process referenced by $pid can not have spawned before $pid referred to the correct process based on the age of the process that $pid points to after the getpidcon() call. (The same thing applies to pretty much any API that refers to processes using PIDs.)

In other words, there is a race condition: The process referred by a given pid can change between the time a transaction was received and the call to getpidcon.

A clean way to fix this design issue is to send the SELinux context with the binder transaction. This is the purpose of the patch we will analyze today.

Patch overview

The patch details were available on lore.kernel.org. The commit title is "create node flag to request sender's security context".

Let's analyze the main part of this patch:

//@@ -3020,6 +3027,20 @@  static void binder_transaction(struct binder_proc *proc,
    t->flags = tr->flags;
    t->priority = task_nice(current);
+   if (target_node && target_node->txn_security_ctx) {
+       u32 secid;
+
+       security_task_getsecid(proc->tsk, &secid);
+       ret = security_secid_to_secctx(secid, &secctx, &secctx_sz);
+       if (ret) {
+           return_error = BR_FAILED_REPLY;
+           return_error_param = ret;
+           return_error_line = __LINE__;
+           goto err_get_secctx_failed;
+       }
+       extra_buffers_size += ALIGN(secctx_sz, sizeof(u64));
+   }

+   if (secctx) {
+       size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) +
+                   ALIGN(tr->offsets_size, sizeof(void *)) +
+                   ALIGN(extra_buffers_size, sizeof(void *)) -
+                   ALIGN(secctx_sz, sizeof(u64));
+       char *kptr = t->buffer->data + buf_offset;
+
+       t->security_ctx = (uintptr_t)kptr +
+           binder_alloc_get_user_buffer_offset(&target_proc->alloc);
+       memcpy(kptr, secctx, secctx_sz);
+       security_release_secctx(secctx, secctx_sz);
+       secctx = NULL;
+   }

When a binder transaction is sent, the kernel checks if the destination process needs the SELinux context (target_node->txn_security_ctx). This option can be specified on binder object init with the flag FLAT_BINDER_FLAG_TXN_SECURITY_CTX.

For instance, the hardware service manager enables this feature:

    //platform/system/hwservicemanager/service.cpp
    int main() {
        // [...]

        sp<ServiceManager> manager = new ServiceManager();
        setRequestingSid(manager, true);
    }

With this feature enabled, the kernel increases the size of extra_buffers_size to store the security context just after. A received transaction looks as following:

Then the receiver can retrieve the security context in the transaction using getCallingPid().

    pid_t IPCThreadState::getCallingPid() const
    {
        return mCallingPid;
    }

    // /platform/system/libhwbinder/IPCThreadState.cpp
    status_t IPCThreadState::executeCommand(int32_t cmd)
    {
    // [...]
    struct binder_transaction_data_secctx {
            struct binder_transaction_data transaction_data;
            binder_uintptr_t secctx;
    };

    binder_transaction_data_secctx tr_secctx;
    binder_transaction_data& tr = tr_secctx.transaction_data;

    if (cmd == (int) BR_TRANSACTION_SEC_CTX) {
        result = mIn.read(&tr_secctx, sizeof(tr_secctx));
    } else {
        result = mIn.read(&tr, sizeof(tr));
        tr_secctx.secctx = 0;
    }
    // ...
    mCallingSid = reinterpret_cast<const char*>(tr_secctx.secctx);

Vulnerability 1: Integer Overflow

A quick review allows to identify an integer overflow.

extra_buffers_size += ALIGN(secctx_sz, sizeof(u64));

extra_buffers_size is fully controlled by user with BC_TRANSACTION_SG transaction. If sum of extra_buffers_size, tr->data_size and tr->offsets_size are equal to zero, the following instructions will copy the security context before t->buffer->data (buf_offset is negative).

    size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) +
                ALIGN(tr->offsets_size, sizeof(void *)) +
                ALIGN(extra_buffers_size, sizeof(void *)) -
                ALIGN(secctx_sz, sizeof(u64));
    char *kptr = t->buffer->data + buf_offset;

    t->security_ctx = (binder_uintptr_t)kptr +
        binder_alloc_get_user_buffer_offset(&target_proc->alloc);
    memcpy(kptr, secctx, secctx_sz);

This vulnerability was identified as CVE-2019-2181 and patched.

It was published in the Android Security Bulletin of September 2019.

Vulnerability 2: Security context overwrite

When a binder transaction contains a BINDER_TYPE_PTR object, the kernel copies the buffer of sender process in the extra part. We can see above that to store the security context, the extra_buffers_size is increased. However the kernel does not make any distinction between standard data and security context in extra buffer. A user can craft a binder transaction which overwrites the security context.

This is an easier way to bypass ACL than the getpidcon() race condition!

This vulnerability was patched by the commit named : binder: Set end of SG buffer area properly

Conclusion

This Binder kernel patch purpose was to improve security and provides another way to retrieve the context of a calling process to avoid usage of getpidcon().

Paradoxically, the initial patch version added more security flaws and made ACL bypass easier than the former vulnerability!