Fix for this issue appeared as CVE-2024-49746: bulletin, patch

"Questo sembra sbagliato"

Il titolo sopra è il commento dal metodo Parcel::continueWrite, che in realtà è responsabile del ridimensionamento degli oggetti Parcel, sia quando esplicitamente richiesto dall'utente (ad esempio tramite setDataSize()) o quando si chiama uno dei metodi write quando la capacità attuale dei dati è troppo piccola```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;

}

[Quando quel commento è stato introdotto](https://android.googlesource.com/platform/frameworks/native/+/53b6ffe5af3951e8784c451ef8c4ff19f3d6b196%5E!/), la chiamata a `closeFileDescriptors()` è stata spostata da `IPCThreadState::freeBuffer()` (che viene chiamata nel codice precedente tramite il puntatore a funzione `mOwner()`) al metodo `continueWrite()`, tuttavia la logica era la stessa di prima. Dopotutto, `Parcel` è una parte fondamentale dell'IPC di Android e se l'IPC principale stesse chiudendo File Descriptors che non dovrebbe, sarebbe un problema ovvio.

Questo ci porta alla parte importante: quando viene utilizzato il codice sopra? Viene utilizzato quando la classe Parcel trasferisce la proprietà dei dati ricevuti dal driver Binder (che a quel punto risiedono in [`/dev/binder` `mmap`](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/native/libs/binder/ProcessState.cpp;l=587-592;drc=187efe18e3de6258af0230198c881915cc695567) e non possono essere scritti (qualsiasi tentativo di scrivere in quella memoria porterebbe a `SIGSEGV`)), cioè quando `Parcel` è o dati di transazione in arrivo (argomento `data` passato a [`onTransact()`](https://developer.android.com/reference/android/os/Binder#onTransact(int,%20android.os.Parcel,%20android.os.Parcel,%20int))) o risposta in arrivo (cioè, l'oggetto `Parcel` che è stato passato alla chiamata [`transact()`](https://developer.android.com/reference/android/os/IBinder#transact(int,%20android.os.Parcel,%20android.os.Parcel,%20int)) come argomento `reply`, `transact()` imposta un riferimento all'interno di quell'oggetto `Parcel`)

In pratica, l'unico caso in cui entreremmo nel blocco `if (mOwner)` è quando il sistema [chiama `setDataSize(0)` per rilasciare i dati di transazione](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/native/libs/binder/IPCThreadState.cpp;l=1483-1488;drc=187efe18e3de6258af0230198c881915cc695567), ma in quel caso entreremmo anche in `if (desired == 0)` che effettua un ritorno anticipato. Durante l'uso legittimo del sistema non c'è alcun caso in cui entreremmo nel percorso "Se c'è un proprietario diverso, dobbiamo prendere possesso"

# Attivare il percorso "take possession"

In uno dei miei precedenti exploit ho mostrato [un caso in cui `createFromParcel()` può effettivamente chiamare `writeInt(0)` su `Parcel` che dovrebbe leggere](https://github.com/michalbednarski/TheLastBundleMismatch#side-effects). Mentre la correzione ha impedito l'esecuzione di qualsiasi metodo `createFromParcel()` non-`Intent` all'interno di `AccountManagerService`, il percorso da `createFromParcel()` a `writeInt(0)` è stato mantenuto intatto.

Per ricapitolare, [all'interno di `PackageParser` abbiamo il seguente codice](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));
}

Pertanto, possiamo avere un oggetto Parcel che è stato passato a createFromParcel passato a qualsiasi costruttore public disponibile nel sistema che accetta un singolo argomento Parcel

E altrove abbiamo il seguente codice:```java public PooledStringWriter(Parcel out) { mOut = out; mPool = new HashMap<>(); mStart = out.dataPosition(); out.writeInt(0); // reserve space for final pool size. }

Pertanto, per attivare il percorso di "prendere possesso", abbiamo bisogno di una chiamata arbitraria a `readParcelable` su `Parcel` che è stata passata a `onTransact()` come `data`; in questo exploit sto usando per quello [lo stesso percorso che ho usato in precedenza in un altro](https://github.com/michalbednarski/LeakValue#putting-parcelables-in-system_server-and-retrieving-them). Ho una chiamata `readParcelable` a `PackageParser$Activity.CREATOR.createFromParcel()`, che a sua volta legge il nome di `PooledStringWriter` e chiama il suo costruttore, e dopo di che i dati di `Parcel` finiscono, quindi `writeInt()` deve riallocare Parcel, entrando così nel nostro percorso di "prendere possesso"

Da notare, se non ci fosse la fine dei dati di `Parcel` a quel punto, `writeInt()` tenterebbe di sovrascrivere i dati sul posto, il che nel caso di dati supportati da `/dev/binder` `mmap` porterebbe a `SIGSEGV`

# Sanitizer dei File Descriptor