Android: Potential Stack Memory Corruption in System Services
Platform: Based on current master in AOSP
Class: Elevation of Privilege

This is a report for your information, based on analysis of the Android code base looking for similar vulnerabilities to my previously reported BnBluetoothGattServer issue. None of the following issues look to be exploitable at the moment for various different reasons but there’s always a chance that in the future they might be so fixing them would be good. For that reason I’ve not placed this under the typical Project Zero 90 day deadline. I’ve submitted this as a security bug but only so it isn’t public until you’ve had a chance to assess its impact. I’ve not looked at any third_party code which might come with Android for similar issues at the moment, so this is just pure android code.

Summary:
I did a basic analysis of buildable Android code looking for variable length arrays being used with untrusted/unbounded lengths. The following are the ones which cause the most concern although each is difficult to exploit for different reasons.

All of the identified issues only allow the stack to grow down. This makes it harder to exploit. However the compiler does not add any checking to determine every page between the current stack base and the destination is accessible so automatic stack growth will fail if the size is large enough causing the stack pointer to alias other memory locations. For example a growth of 32MiB is usually sufficient to cause the stack to stop growing. If a memory leak/heap flood primitive can be found in a service then the stack size can be constrained easily to < 1MiB which makes these far more exploitable (on 32 bit of course, 64 bit you wouldn’t be able to do this)

frameworks/av/media/img_utils/src/DngUtils.cpp:
https://android.googlesource.com/platform/frameworks/av/+/master/media/img_utils/src/DngUtils.cpp#66

float redMap[lsmWidth * lsmHeight];
float greenEvenMap[lsmWidth * lsmHeight];
float greenOddMap[lsmWidth * lsmHeight];
float blueMap[lsmWidth * lsmHeight];

Massive buffers depending on data which could come from an untrusted source. This is probably unlikely to be exploitable, it would only affect something in process but it is used for capturing camera images so might affect camera service in certain scenarios.

platform/system/gatekeeper/gatekeeper.cpp
https://android.googlesource.com/platform/system/gatekeeper/+/master/gatekeeper.cpp#184

uint8_t to_sign[password_length + metadata_length];

The field password_length is the length of the password coming from the IPC call. Due to binder size limits this probably not possible to extend to more than around the 1MiB size. Also this would require a permission anyway.

platform/system/security/keystore/keystore.cpp
https://android.googlesource.com/platform/system/security/+/master/keystore/keystore.cpp#120
https://android.googlesource.com/platform/system/security/+/master/keystore/keystore.cpp#126
https://android.googlesource.com/platform/system/security/+/master/keystore/keystore.cpp#132

char encoded[encode_key_length(keyName) + 1];

keyName is passed from the keystore IPC. Again it looks like IPC size limits would hinder this, although it does its best to help by converting UTF16 to UTF8 (giving a 2->3 increase) then for every non ascii character this again is doubled so encode_key_length will max out at about 1.7MiB which is within range if we could fill up service memory. Again it isn’t exploitable by luck more than anything, and if the binder was ever to change in the future to allow larger values this could become vulnerable.

frameworks/native/libs/binder/IProcessInfoService
https://android.googlesource.com/platform/frameworks/native/+/master/libs/binder/IProcessInfoService.cpp#72

size_t len = static_cast<size_t>(arrayLen);
int32_t pids[len];

This would almost certainly be potentially exploitable as the length is directly read from the Parcel and used with only a <= 0 check. Fortunately it doesn’t look like this code is actually used, instead the IProcessInfoService service is implemented in the Activity Manager in Java. I guess this might be something which in the future will be used?

You might want to think about banning the use of variable length arrays entirely (there are more instances than this, but then don’t look to be able to be exploited).

I’ve not provided a PoC for any of these take, as is. And as mentioned this information is not provided under the 90 day disclosure deadline.