Android - Inter-Process munmap due to Race Condition in ashmem 2018-01-08 17:05:03

The MemoryIntArray class allows processes to share an in-memory array of integers backed by an "ashmem" file descriptor. As the class implements the Parcelable interface, it can be inserted into a Parcel, and optionally placed in a Bundle and transferred via binder to remote processes.

Instead of directly tracking the size of the shared memory region, the MemoryIntArray class calls the ASHMEM_GET_SIZE ioctl on the ashmem descriptor to retrieve it on-demand. Previously, the code made a single call to ASHMEM_GET_SIZE in order to retrieve the region's size, both before mapping it, and before unmapping it. Since the region's size could be set via ASHMEM_SET_SIZE until the region has been mapped, this opened the possibility for race conditions where an attacker alters the size in-between the first size retrieval and the mapping operation.

This issue has since been addressed (CVE-2017-0412), using the following pattern:
(see http://androidxref.com/8.0.0_r4/xref/frameworks/base/core/jni/android_util_MemoryIntArray.cpp#69)

1. int ashmemSize = ashmem_get_size_region(fd);
2. if (ashmemSize <= 0) {
3. jniThrowException(env, "java/io/IOException", "bad ashmem size");
4. return -1;
5. }
7. // IMPORTANT: Ashmem allows the caller to change its size until
8. // it is memory mapped for the first time which lazily creates
9. // the underlying VFS file. So the size we get above may not
10. // reflect the size of the underlying shared memory region. Therefore,
11. // we first memory map to set the size in stone an verify if
12. // the underlying ashmem region has the same size as the one we
13. // memory mapped. This is critical as we use the underlying
14. // ashmem size for boundary checks and memory unmapping.
15. int protMode = owner ? (PROT_READ | PROT_WRITE) : PROT_READ;
16. void* ashmemAddr = mmap(NULL, ashmemSize, protMode, MAP_SHARED, fd, 0);
17. if (ashmemAddr == MAP_FAILED) {
18. jniThrowException(env, "java/io/IOException", "cannot mmap ashmem");
19. return -1;
20. }
22. // Check if the mapped size is the same as the ashmem region.
23. int mmapedSize = ashmem_get_size_region(fd);
24. if (mmapedSize != ashmemSize) {
25. munmap(reinterpret_cast<void *>(ashmemAddr), ashmemSize);
26. jniThrowException(env, "java/io/IOException", "bad file descriptor");
27. return -1;
28. }

As we can see above, the code verifies that the size retrieved prior to mapping and after performing the mapping operation are equal, thus attempting to eliminate the race condition. However, looking at the ashmem driver, the following code is used to implement the ASHMEM_SET_SIZE ioctl:
(see http://androidxref.com/kernel_3.18/xref/drivers/staging/android/ashmem.c#753)

b. ret = -EINVAL;
c. if (!asma->file) {
d. ret = 0;
e. asma->size = (size_t) arg;
f. }
g. break;

The ioctl does not acquire the "ashmem_mutex" to perform the ioctl itself. Therefore, an "mmap" operation could be in-flight, while the ASHMEM_SET_SIZE ioctl is being processed. This opens up the possibility to the following schedule, triggering a race condition:

[Process A]:
1. Attacker sends a MemoryIntArray with a crafted ashmem file descriptor in a Bundle, and with a small size

[System Server]:
2. Target process (e.g., system_server) unparcels the bundle with the MemoryIntArray, instantiating it
3. This triggers the code path above, executing lines 1-16

[Process A]:
4. Attacker calls ASHMEM_SET_SIZE, either during or before the mmap call
4.1. Lines a-c are executed, asma->file is still NULL

[System Server]:
5. Target process continues executing lines 16-24
5.1. Target process sees the old size, as the ASHMEM_SET_SIZE operation didn't complete yet
5.2. Therefore, the condition at line 24 is not satisfied

[Process A]:
6. Lines d-f are executed, setting the size to a new value

[System Server]:
7. Some time later, target process runs the finalizer, which retrieves the new size, and uses it to munmap the descriptor
7.1. This causes an inter-process munmap with an attacker-controller size

This issue can be exploited similarly to the previous ashmem bugs -- once a larger "munmap" is performed in the target process, it can be used to "free" a data structure such as a thread's stack, allowing the attacker to replace it with their own controlled contents.

While the exploitable condition is present in MemoryIntArray, I believe a fix should also be applied to the kernel to prevent such conditions from occurring in other contexts. Namely, the ashmem driver should acquire the "ashmem_mutex" during the ASHMEM_SET_SIZE operation, in order to guarantee that no races with ongoing "mmap" operations are possible. In addition, MemoryIntArray should not rely on multiple calls to ASHMEM_GET_SIZE, but should rather perform a single ASHMEM_GET_SIZE operation and store the returned size for both the "mmap" and "munmap" operations.

To demonstrate the race condition, I've added a busy loop to the ashmem driver between lines c. and d., increasing the race window to allow for easier demonstration of the schedule above.

I've attached a PoC which triggers this race condition and causes system_server to call munmap on a large memory region. To reproduce the issue, apply the diff in "ashmem_delay.diff" to the ashmem driver, then run the attached program. Doing so should result in a large "munmap" operation in system_server, causing it to crash.

The issue can also be exploited from the "isolated_app" SELinux context (and perhaps from the Chrome sandbox?), as all that's required to leverage the attack is the ability to issue ashmem syscalls, and to interact with the ActivityManager service (which is exposed to "isolated_app").

Proof of Concept: