La corrección para este problema apareció como CVE-2024-49746: boletín, parche

"Esto parece incorrecto"

El título anterior es el comentario del método Parcel::continueWrite, que en realidad es el responsable de redimensionar los objetos Parcel, ya sea cuando el usuario lo solicita explícitamente (por ejemplo a través de setDataSize()) o al llamar a uno de los métodos write cuando la capacidad de datos actual es demasiado pequeña```cpp status_t Parcel::continueWrite(size_t desired) { // SNIP: Validate desired size // SNIP: Assign kernelFields & rpcFields from variant member of this class // SNIP: Count number of objects (Binder handles and File Descriptors) // that will be present after resize and assign to objectsSize

if (mOwner) {
    // If the size is going to zero, just release the owner's data.
    if (desired == 0) {
        freeData();
        return NO_ERROR;
    }

    // If there is a different owner, we need to take
    // posession.
    uint8_t* data = (uint8_t*)malloc(desired);
    // SNIP: Check if malloc succeeded
    binder_size_t* objects = nullptr;

    if (kernelFields && objectsSize) {
        objects = (binder_size_t*)calloc(objectsSize, sizeof(binder_size_t));
        // SNIP: Check if calloc succeeded

        // Little hack to only acquire references on objects
        // we will be keeping.
        size_t oldObjectsSize = kernelFields->mObjectsSize;
        kernelFields->mObjectsSize = objectsSize;
        acquireObjects();
        kernelFields->mObjectsSize = oldObjectsSize;
    }
    // SNIP: rpcFields handling for non-/dev/binder Parcels

    if (mData) {
        memcpy(data, mData, mDataSize < desired ? mDataSize : desired);
    }
    if (objects && kernelFields && kernelFields->mObjects) {
        memcpy(objects, kernelFields->mObjects, objectsSize * sizeof(binder_size_t));
    }
    // ALOGI("Freeing data ref of %p (pid=%d)", this, getpid());
    if (kernelFields) {
        // TODO(b/239222407): This seems wrong. We should only free FDs when
        // they are in a truncated section of the parcel.
        closeFileDescriptors();
    }
    mOwner(mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr,
           kernelFields ? kernelFields->mObjectsSize : 0);
    mOwner = nullptr;

    // SNIP: Allocation count tracking
    // SNIP: Assign data and objects to this object
} else if (mData) {
    // SNIP: Resize data owned by this instance of Parcel
} else {
    // SNIP: Allocate initial data for currently empty Parcel
}

return NO_ERROR;

}

[Cuando se introdujo ese comentario](https://android.googlesource.com/platform/frameworks/native/+/53b6ffe5af3951e8784c451ef8c4ff19f3d6b196%5E!/), la llamada a `closeFileDescriptors()` se movió de `IPCThreadState::freeBuffer()` (que se invoca en el código anterior a través del puntero de función `mOwner()`) al método `continueWrite()`, aunque la lógica era la misma que antes. Después de todo, `Parcel` es una parte central del IPC de Android y si el IPC central estuviera cerrando descriptores de archivo que no debería, sería un problema evidente

Lo que nos lleva a la parte importante: ¿cuándo se utiliza el código anterior? Se utiliza cuando la clase `Parcel` transfiere la propiedad de los datos recibidos del controlador de Binder (que en ese momento residen en el [`mmap` de `/dev/binder`](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/native/libs/binder/ProcessState.cpp;l=587-592;drc=187efe18e3de6258af0230198c881915cc695567) y no se puede escribir en ellos (cualquier intento de escribir en esa memoria provocaría un `SIGSEGV`)), es decir, que `Parcel` sea o bien los datos de transacción entrantes (el argumento `data` pasado a [`onTransact()`](https://developer.android.com/reference/android/os/Binder#onTransact(int,%20android.os.Parcel,%20android.os.Parcel,%20int))) o bien la respuesta entrante (es decir, el objeto `Parcel` que se pasó a la llamada [`transact()`](https://developer.android.com/reference/android/os/IBinder#transact(int,%20android.os.Parcel,%20android.os.Parcel,%20int)) como argumento `reply`; `transact()` establece la referencia dentro de ese objeto `Parcel`)

En la práctica, el único caso en el que entraríamos en el bloque `if (mOwner)` es cuando el sistema [llama a `setDataSize(0)` para liberar los datos de transacción](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/native/libs/binder/IPCThreadState.cpp;l=1483-1488;drc=187efe18e3de6258af0230198c881915cc695567), pero en ese caso también entraríamos en `if (desired == 0)`, que hace un retorno anticipado. Durante el uso legítimo del sistema no existe ningún caso en el que entremos en la ruta "Si hay un propietario distinto, debemos tomar posesión"

# Activar la ruta "tomar posesión"

En uno de mis exploits anteriores mostré el [caso en el que `createFromParcel()` puede realmente llamar a `writeInt(0)` sobre el `Parcel` del que debería estar leyendo](https://github.com/michalbednarski/TheLastBundleMismatch#side-effects). Si bien la corrección aplicada allí impidió la ejecución de cualquier método `createFromParcel()` que no fuera de `Intent` dentro de `AccountManagerService`, la ruta de `createFromParcel()` a `writeInt(0)` se mantuvo intacta

Para recapitular, [dentro de `PackageParser` tenemos el siguiente código](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/content/pm/PackageParser.java;l=7789-7795;drc=7d3ffbae618e9e728644a96647ed709bf39ae759):```java
final Class<T> cls = (Class<T>) Class.forName(componentName);
final Constructor<T> cons = cls.getConstructor(Parcel.class);

intentsList = new ArrayList<>(N);
for (int i = 0; i < N; ++i) {
    intentsList.add(cons.newInstance(in));
}

Por lo tanto, podemos tener un objeto Parcel que se pasó a createFromParcel pasado a cualquier constructor public disponible en el sistema que acepte un único argumento Parcel

Y en otro lugar tenemos el siguiente código:```java public PooledStringWriter(Parcel out) { mOut = out; mPool = new HashMap<>(); mStart = out.dataPosition(); out.writeInt(0); // reserve space for final pool size. }

Por lo tanto, para activar la ruta de "take possession", necesitamos tener una llamada arbitraria a `readParcelable` en un `Parcel` que se pasó a `onTransact()` como `data`; en este exploit estoy usando para eso [la misma ruta que usé previamente en otro](https://github.com/michalbednarski/LeakValue#putting-parcelables-in-system_server-and-retrieving-them). Hago que `readParcelable` llame a `PackageParser$Activity.CREATOR.createFromParcel()`, que a su vez lee el nombre de `PooledStringWriter` y llama a su constructor; después de eso, los datos del `Parcel` terminan, por lo que `writeInt()` necesita reasignar el Parcel, entrando en nuestra ruta de "take possession".

Cabe señalar aquí que, si no hubiera fin de los datos de `Parcel` en ese punto, `writeInt()` intentaría sobrescribir los datos en el lugar, lo que, en el caso de datos respaldados por `/dev/binder` `mmap`, provocaría un `SIGSEGV`.

# File Descriptor Sanitizer