How a double-free bug in WhatsApp turns to remote code execution
awakened1712.github.io
awakened1712.github.io
Another victim of Android not exposing the image codecs to the NDK.
I guess, like many, they decided to use libpl_droidsonroids_gif instead of having to deal with JNI to call BitmapFactory or ImageDecoder.
Because of these kind of exploits codecs have their own hardness processes, https://source.android.com/devices/media/framework-hardening, but naturally it doesn't help when applications bring their own native alternatives along instead.
Because that is what that tone reminds me of.
"so you're again looking for reasoning in the wrong bush"
I get that freelists are fast and great for small chunk reuse, (I'm guessing that's the implementation that allows double return) but is there no way to protect against double-free here? I wish it crashed instead.
Edit: it does indeed look like a form of a freelist - https://blog.nsogroup.com/a-tale-of-two-mallocs-on-android-l...
Compiler - insert code and memory usage to show area as free/not-free and check on malloc/free. Data has to be recorded at runtime because the type system does not provide enough information to be sure. Static analysis can only guess with C.
OS - Every malloc would need to be in a separate page
Libc - could do something similar to compiler
They all come with overhead in performance and/or memory usage, unless you change the technology stack in some way. Garbage collection is one approach. Adding extra type information to make it clear from static analysis (so you get something like Rust) is another. Or, some hardware support that gives more granular permissions.
You could also xor the memory location into the marker to avoid multiple collisions for applications that allocate a large amount of similar objects.
Memory allocations vary in size (obviously). The simplest algorithm would be to take the block of memory and start allocating chunks from the beginning. The problem is that if your app does a lot of small allocations, frees them and then wants to do a large allocation, the free list might not contain a block large enough. The job of your allocator is to manage that fragmentation. There are a lot of approaches, but fundamentally at some point it has to put two deallocations 'back together' to make a larger one.
If you free a chunk, and then that chunk is coalesced with some data just before it, then the free list will no longer contain a pointer that matches the one that you then try to double free.
I think it also would make the memory manager less cache-friendly, as reusing just freed memory blocks would be impossible.
https://security.googleblog.com/2019/08/adopting-arm-memory-...
https://crates.io/crates/image
It should be straightforward to swap out the vulnerable C library with a thin wrapper around the above crate.
It's true that there are costs associated with using Rust in the build process and so forth, but WhatsApp is trying to be a secure messenger and is FAANG-backed. The challenges can be overcome.
If that's an issue, I bet the Rust community is interested in making it easier and better supported.
This are the expectations that any external language should meet versus what platform languages offer out of the box.
While the experience is still found lacking versus what iOS or UWP tooling are capable of, it still is much better than any third party language integration.
Also, it's called the "Android NDK" and not the "Android C SDK" - a big part of what the NDK gives you is not C-specific. It makes almost no difference whether you compile your code with GCC vs rustc, at the end of the day you're creating a native shared library and loading it from a java application.
For example, these instructions from 2017 show how little work is involved https://mozilla.github.io/firefox-browser-architecture/exper... and support has only improved since then.
Using static analysis tooling on C/C++ code is more work than the above to set up, so I really do believe that Rust is the easiest way to reduce the chance of these kinds of bugs happening again.
Yes it is called the NDK and the workflow that you describe doesn't support the bullet points I described.
If Rust wants to be embraced by Android developers it needs to up its game in Android Studio tooling, Binder generation and FFI integration.
Those instructions from 2017 are completely outdated given Android 3.5 and NDK r20.
GCC is no longer around, C++ support has improved quite substantially, static analysis tooling is integrated into NDK, Android Studio 3.7 (planned for next year) will bring support for mixed mode NDK libraries in AAR format.
All memory allocations are now misaligned, decreasing performance. On some platforms misaligned access causes a bus error exception.
The marker would generally need to be at least 8 bytes. This would alleviate the problems, but also cause pretty significant overhead.
#define my_free(x) { free(x); (x) = NULL; }
Note that the extra assignment will often be optimized away because
(a) the pointer is assigned to a malloc() after the call to my_free() OR
(b) the pointer goes out of scope e.g. is popped off the stack.
Generally I agree with you though, setting pointers to NULL after free is good practice and probably worth enforcing in the coding style.
https://security.googleblog.com/2019/05/queue-hardening-enha...
(That is, assuming asan would have really captured this specific instance).
Not that I don't appreciate sanitizers, I just used asan to find the cause of a memory corruption bug just a couple of hours ago and it was great. In the past I would have used valgrind which is significantly slower.
(as a _very_ crude approximation, with UBSan on, you have the performance of Java or Go, and with the other sanitizers you get closer to Ruby territory)
You can't combine the sanitizers anyway. They're really designed to be enabled for a testsuite and debug builds.
The exploit works well for Android 8.1 and 9.0, but does not work for Android 8.0 and below. In the older Android versions, double-free could still be triggered. However, because of the malloc calls by the system after the double-free, the app just crashes before reaching to the point that we could control the PC register.
tangent: Your "About" link goes to a 404: https://awakened1712.github.io/about/
>This issue affects WhatsApp for iOS before version v2.19.100 and WhatsApp for Android before version 2.19.243.
But the latest version I see on the App Store is 2.19.92 (iPhone S3, iOS 13). The AppStore website says the same[1].
The Android version[2] seems updated (2.19.271)
[0]: https://www.facebook.com/security/advisories/cve-2019-11927
[1]: https://apps.apple.com/in/app/whatsapp-messenger/id310633997
My understanding is: with the double-free, they managed to overwrite "info", including the location of the function "rewindFunction". When the parsing process calls that function, it will therefore actually call whatever function the attacker has pointed "rewindFunction" to.
IMO ROP gadgets are terribly clever. Since you can't just execute new malicious code but you do have control over which instruction to execute next, you have to scour the executable and its libraries for any content that would have the effects that you require.
Take a look at the output from ROPGadget [1] (look at "ROP Chain Generation" screenshot) to see an example of how it works.
[1] https://www.openbsd.org/papers/asiabsdcon2019-rop-paper.pdf
The lib in question parses the GIF twice.
In the first run, it allocs an internal info struct of size X, then you trigger the double-free so this info struct is freed twice.
In the second run, it allocs the same internal info struct of size X and gets a ptr Y. Then, by crafting the GIF so one of the frames to be decoded is also of the size X, it will alloc intermediary space for this frame, but due to it being the same size, you'll get the same ptr Y returned, and the frame gets unpacked over the info structure...
So, you get your user-provided frame data placed in the internal info struct. Luckily for the exploiter, there are function pointers inside the info struct which are called a bit later.
So you can provide a memory address to jump to by putting it in the right place in the magic frame. You can't also put executable code in the magic frame due to restrictions, but you can place shell commands in the frame and shuffle registers by using already available executable chunks by selecting the right jump address (this is explained well in the post I think).
As in network server hardening, you can usually start by following where user-provided data goes, and harden its path.
In this case however the first problem was a clandestine bug (the double free in some cases), probably induced by the programmer trying to be a bit too clever with the management of these structs (if you're not threaded you could simply have a single static declaration for this info struct and forget the malloc/freeing, or at least malloc it just once upon each thread init, haven't looked at this code..).
Generally keeping track of all malloc/free pairs is tricky and you can go a long way by trying to simplify your logic, I mean even if you don't get exploited like this, you might simply crash sometime or leak memory or in general behave badly.
There are good reasons why there exist all kinds of memory-managed languages :)
However, reading https://github.com/aseprite/giflib/blob/master/lib/openbsd-r..., I don’t see how that could lead to a double-free, unless realloc double-frees, or unless a different reallocatearray gets linked in.
Also, that comment on how realloc isn’t portable feels scary. I can see that introduce subtle bugs in libraries used on a different platform from where it is developed.
Hence, I think one should forbid the use of raw ‘realloc’ in portable code.
realloc on linux frees the ptr https://linux.die.net/man/3/realloc
https://github.com/koral--/android-gif-drawable/blob/dev/and...
Or perhaps the function from the libc was being used?
int_fast32_t widthOverflow = gifFilePtr->Image.Width - info->originalWidth;
int_fast32_t heightOverflow = gifFilePtr->Image.Height - info->originalHeight;
const uint_fast32_t newRasterSize =
gifFilePtr->Image.Width * gifFilePtr->Image.Height;
if (newRasterSize > info->rasterSize || widthOverflow > 0 ||
heightOverflow > 0) {
...
info->rasterSize = newRasterSize;
OT, but isn't that test redundant? For rasterSize = height x width to increase, at least one of height or width must increase, and so anytime the first term of the || is true, at least one of the other terms will also be true, so the first term is redundant. It seems it could be simply this: int_fast32_t widthOverflow = gifFilePtr->Image.Width - info->originalWidth;
int_fast32_t heightOverflow = gifFilePtr->Image.Height - info->originalHeight;
if (widthOverflow > 0 || heightOverflow > 0) {Not if they're negative.
And maybe if Signal has a similar issue? I'm not sure if they use the same GIF decoding library on Android? And iOS?
If someone doesn't know off the top of their head maybe they can point me to some docs.
https://issuetracker.google.com/issues/128554619
As of Android Q you can no longer execute native binaries that aren't shipped in the .apk, is that related? (not a systems dev, hence the question)
As someone who has also written a GIF decoder, more for learning purposes than anything, I checked what mine would do with that GIF: it does reallocate twice, but since the first time already nulls the buffer pointer, it doesn't actually double-free (since free()'ing a NULL pointer is defined by the standard to have no effect.)
Do I have to do this myself? Doesn't update itself?
> The exploit works well until WhatsApp version 2.19.230. The vulnerability is official patched in WhatsApp version 2.19.244
At least from my experience breaking stuff for security lectures/CTFs.
This is probably also a good way to make a name in the security industry (RCE against WA on CV should look pretty neat). [edit]So not getting paid is relative (and again, if happens out of curiosity - not a huge difference spending a weekend binging some anime or breaking stuff).[/edit]
A simple (number_of_words)/(200WPM) isn't a great metric when words from code snippets are also counted.
notroot@osboxes:~/Desktop/gif$ ./exploit
buffer = 0x7ffc586cd8b0 size = 266
47 49 46 38 39 61 18 00 0A 00 F2 00 00 66 CC CC
FF FF FF 00 00 00 33 99 66 99 FF CC 00 00 00 00
00 00 00 00 00 2C 00 00 00 00 08 00 15 00 00 08
9C 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 84 9C 09 B0
C5 07 00 00 00 74 DE E4 11 F3 06 0F 08 37 63 40
C4 C8 21 C3 45 0C 1B 38 5C C8 70 71 43 06 08 1A
34 68 D0 00 C1 07 C4 1C 34 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 54 12 7C C0 C5 07 00 00 00 EE FF FF 2C 00 00
00 00 1C 0F 00 00 00 00 2C 00 00 00 00 1C 0F 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 2C 00 00 00 00
18 00 0A 00 0F 00 01 00 00 3B