Patch misterioso

Iniziamo questa volta con la patch apparsa come fix per CVE-2023-45777 nel Android Security Bulletin:```diff diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java index 7a19d034c2c8..5238595fe2a2 100644 --- a/services/core/java/com/android/server/accounts/AccountManagerService.java +++ b/services/core/java/com/android/server/accounts/AccountManagerService.java @@ -4923,7 +4923,7 @@ public class AccountManagerService p.setDataPosition(0); Bundle simulateBundle = p.readBundle(); p.recycle();

•        Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT);
  

•        Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT, Intent.class);
         if (intent != null && intent.getClass() != Intent.class) {
             return false;
         }
  

Poche persone erano abbastanza perplesse da chiedermelo; in precedenza ho risposto loro con alcuni suggerimenti e ora pubblico il writeup completo per questo problema

Ma prima forniamo un po' di contesto su cosa sta succedendo in questa patch

Questo è un cambiamento nel metodo [`checkKeyIntent()`](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/services/core/java/com/android/server/accounts/AccountManagerService.java;l=4938-4954;drc=47de64a38aa1799cb41f41b2ea0c539ee61de64d). Questo metodo esegue più controlli per garantire che l'`Intent` fornito dall'applicazione sia sicuro per il sistema da lanciare (usando i privilegi del sistema)

Innanzitutto, questo metodo usa `checkKeyIntentParceledCorrectly()` che serializza e deserializza nuovamente il `Bundle` che stiamo controllando e verifica se l'`Intent` preso dal `Bundle` prima di ciò corrisponde all'`Intent` dal `Bundle` dopo tale ciclo. Poiché il lancio dell'`Intent` avviene in altri processi di app di sistema diversi da quello che esegue la validazione, in precedenza era [possibile costruire `Bundle` che apparivano sicuri durante la validazione all'interno di `AccountManagerService`, ma contenevano un `Intent` diverso dopo essere stati inviati al processo successivo](https://github.com/michalbednarski/IntentsLab/issues/2#issuecomment-344365482). Questo simula l'invio del `Bundle` al processo successivo per rilevare tali situazioni.

Dopo `checkKeyIntentParceledCorrectly()` abbiamo la chiamata `bundle.getParcelable()`, che questa patch sostituisce dalla versione deprecata che poteva costruire qualsiasi oggetto a una che valida che l'oggetto che sta per essere deserializzato sia del tipo specificato nel secondo parametro

Quella versione con il parametro di tipo è stata introdotta in Android 13, come parte di un più ampio rafforzamento di `Parcel`/`Bundle`. In particolare, prima di Android 13, quando un `Bundle` veniva inviato tra processi, manteneva una copia grezza dell'intero dato serializzato finché non veniva acceduto a qualsiasi elemento, momento in cui ogni valore veniva deserializzato. Ora, quando un valore viene acceduto per la prima volta dopo che il `Bundle` è stato ricevuto, vengono deserializzate solo le chiavi `String` e i valori dei tipi primitivi, mentre i valori non primitivi vengono lasciati come `LazyValue`, che hanno la loro lunghezza memorizzata come parte del dato serializzato per garantire che anche quando la logica di serializzazione/deserializzazione non corrisponde, tali discrepanze non influenzino altre voci

Prima di addentrarci, diamo un'occhiata a `LazyValue`: nel suo codice sorgente [abbiamo un bel commento che spiega la sua struttura dati](https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/os/Parcel.java;l=4392-4399;drc=03c34f57c05feecfb090de3917787f049cb5f804)```
                     |   4B   |   4B   |
mSource = Parcel{... |  type  | length | object | ...}
                     a        b        c        d
length = d - c
mPosition = a
mLength = d - a

mPosition e mLength descrivono la posizione dell'intero dato LazyValue nel Parcel originale, inclusi type e length. "length" (senza "m" all'inizio) si riferisce al valore di lunghezza come scritto nel Parcel ed esclude l'header (type e length)

Se un Bundle contenente un LazyValue viene inoltrato a un altro processo, l'intero LazyValue inclusi i campi type e length viene copiato verbatim da Bundle.mParcelledData al Parcel di destinazione

Quando si accede all'elemento del Bundle rappresentato da LazyValue, il Parcel viene riavvolto a mPosition e viene chiamato readValue(). Se viene passato un argomento di tipo a bundle.getParcelable(), questo viene propagato a readValue() che garantirà sia che il tipo in procinto di essere de-parcellizzato sia quello atteso, sia che dopo la de-parcellizzazione venga verificato che il tipo del valore de-parcellizzato sia quello atteso. Dopo la de-parcellizzazione, LazyValue viene sostituito, così la volta successiva che il Bundle viene scritto nel Parcel, il valore verrà serializzato di nuovo tramite writeValue()

L'uso del parametro tipizzato Bundle.get*()/Parcel.read*() è per lo più rilevante per metodi come Parcel.readParcelableList(), che restituisce un ArrayList e, a causa della Type Erasure di Java, anche se facessi qualcosa come List<SomeParcelableType> field = parcel.readParcelableList();, la parte <SomeParcelableType> non verrebbe applicata a runtime e tale List potrebbe contenere qualsiasi classe Parcelable disponibile nel sistema, quindi tutti i createFromParcel/writeToParcel disponibili nel sistema potrebbero essere usati come parte della serializzazione/deserializzazione del tipo che conteneva tale List

Potresti anche voler dare un'occhiata alla presentazione del team Android Security and Privacy sull'introduzione di questi meccanismi (slide, video)

Qui, tuttavia, l'uso della versione tipizzata sembra ridondante, poiché controlliamo anche esplicitamente il tipo dell'oggetto restituito. Allora cosa sta succedendo e quale vulnerabilità viene corretta qui?

Effetti collaterali

Riguarda la patch dall'inizio