
How a double-free bug in WhatsApp turns to remote code execution - wyldfire
https://awakened1712.github.io/hacking/hacking-whatsapp-gif-rce/
======
pjmlp
> WhatsApp parses it with a native library called libpl_droidsonroids_gif.so
> to generate the preview of the GIF file.

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](https://source.android.com/devices/media/framework-hardening), but
naturally it doesn't help when applications bring their own native
alternatives along instead.

~~~
izacus
ImageDecoder didn't support animated gif for all APIs they support so you're
again looking for reasoning in the wrong bush - exposing codecs wouldn't help.

~~~
pjmlp
So I should look into careless programming, lack of fuzzing and proper testing
practices bush instead?

Because that is what that tone reminds me of.

~~~
mentat
Your proposed solution wouldn't work. You appear to have been unaware and
instead of saying "thanks, I didn't realize the limitation existed, that's
unfortunate" you instead attack the messenger. All file parsers should be
throughly fuzzed, yes. Not sure what tone has to do with it.

~~~
pjmlp
That would have been indeed my answer if the comment didn't look like a snarky
attack.

 _" so you're again looking for reasoning in the wrong bush"_

~~~
thenewnewguy
[https://rationalwiki.org/wiki/Tone_argument](https://rationalwiki.org/wiki/Tone_argument)

------
viraptor
> In Android, a double-free of a memory with size N leads to two subsequent
> memory-allocation of size N returning the same address.

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...](https://blog.nsogroup.com/a-tale-of-two-mallocs-on-android-libc-
allocators-part-1-dlmalloc/)

~~~
rkangel
At what level are you looking to cause the crash? Some options:

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.

~~~
mixedbit
Perhaps malloc() could allocate 1 byte extra at the start of the allocated
block with some magic number written to it and free() could assert that the
number is correct and change the number to mark the block as freed.

~~~
vardump
Problems: double free might still happen, except now some innocent memory
block can be corrupted as well, which just happened to have the magic value in
that address.

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.

~~~
laughinghan
On the other hand, the chances of an innocent memory block having a random
8-byte magic value is vanishingly small, so that would work even better. If
free() took only 1ns to run and you called it 2^64 times, that would take >500
years.

------
mgrviper
At first i was wondering how one can get RCE out of double-free and then
author proceed to drop a bomb - android would reliably return same adress to
the next two allocations of same size as freed memory. Android behaviour here
is simply unacceptable. One would expect (yeah) memory managment bugs from
user space applications, but return same memory from a default allocator twice
because of double-free is a terrible peculiarity, undefined behavour or not.

~~~
umanwizard
How do other malloc implementations avoid this? It seems natural if what
“free” does involves adding the pointer to some free list. Obviously you
wouldn’t want to scan the whole free list every time looking for duplicates -
is there another way to avoid this behavior?

~~~
tedunangst
Bitmaps don't require scanning.

~~~
saagarjha
They do, you'd just scanning a smaller thing.

~~~
tedunangst
What are you scanning the bitmap looking for? Why not just index into it and
look at the relevant bit?

------
sebastianconcpt
_Affected versions The exploit works well until WhatsApp version 2.19.230. The
vulnerability is official patched in WhatsApp version 2.19.244

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._

------
danso
Even as someone who hasn't had to deal with memory allocation since college CS
classes many years ago, I found this explainer to be easy to follow and
enlightening. Well done!

tangent: Your "About" link goes to a 404:
[https://awakened1712.github.io/about/](https://awakened1712.github.io/about/)

~~~
jor-el
The author of the blog post is my friend, I will ask him to fix it. The funny
thing is, he don't even knows about HN at all and he has no idea that his post
is trending. :P

------
captn3m0
Somewhat related question: Does anyone know when the fixes for CVE-2019-11927
will be released for iOS? The advisory[0] says:

>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](https://www.facebook.com/security/advisories/cve-2019-11927)

[1]: [https://apps.apple.com/in/app/whatsapp-
messenger/id310633997](https://apps.apple.com/in/app/whatsapp-
messenger/id310633997)

[2]: [https://www.whatsapp.com/android/](https://www.whatsapp.com/android/)

~~~
jannes
Whenever Apple finishes the app review, I suppose. It's a bit of a shame that
people are running vulnerable versions of WhatsApp because Apple is taking its
time.

~~~
tinus_hn
There is an expedited review process for this kind of thing. It’s impossible
to tell who is to blame here. It could be that they abused the expedited
review process before and are excluded, or they never requested it, or perhaps
they were late in sending in the new version. Or Apple found a problem in the
new version, or they were slow.

------
wil421
Can someone explain the RCE part? I understand the double free bug but not the
exploit part.

~~~
l33tman
The double-free bug means that a double-free of a chunk of size X might lead
to subsequent allocs of that size X to return the same pointer.

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).

~~~
lubesGordi
Awesome explanation! Thank you! Does this mean also that function pointers
inherently weaken security (by providing a means of code execution given other
faults)?

~~~
l33tman
They are a very common design-pattern in traditional C, as you kind of emulate
object-oriented virtual function overloading by using them in structs. I don't
think it's realistic to avoid them just by principle..

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
:)

------
Someone
If I understand this correctly, the underlying problem is in GIFLib, which
calls _reallocatearray_ , a wrapper around _realloc_ that guards against
overflows when computing the size of the memory buffer to reallocate from the
number of items and item size.

However, reading
[https://github.com/aseprite/giflib/blob/master/lib/openbsd-r...](https://github.com/aseprite/giflib/blob/master/lib/openbsd-
reallocarray.c), 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.

~~~
totony
Most likely this is getting linked
[https://code.woboq.org/userspace/glibc/malloc/reallocarray.c...](https://code.woboq.org/userspace/glibc/malloc/reallocarray.c.html)

realloc on linux frees the ptr
[https://linux.die.net/man/3/realloc](https://linux.die.net/man/3/realloc)

~~~
jwilk
I thought Adroid had its own BSDish libc, not glibc? (But the implementation
is likely mostly-identical anyway.)

~~~
saagarjha
Android uses its own implementation, called Bionic:
[https://en.wikipedia.org/wiki/Bionic_(software)](https://en.wikipedia.org/wiki/Bionic_\(software\))

------
pratio
The post is impressive with the demo and explanations

------
daenz
That was surprisingly easy to follow. I always thought these exploits were
quite a bit more magical, but it seems pretty straightforward. Scary.

------
tzs

      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) {

~~~
saagarjha
> rasterSize = height x width to increase, at least one of height or width
> must increase

Not if they're negative.

~~~
tzs
height and width are of type GifWord, which is typedefed to uint_fast16_t.

------
swagonomixxx
Does anyone know if such behavior is possible on iOS as well?

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.

------
gok
Is there a legitimate reason for the Android sandbox to allow calls to
`system()`?

~~~
archi42
I suppose in case you ship some prebuilt executable to do stuff? E.g. ffmpeg.
Just a guess, though.

~~~
asveikau
IMO execve with its already-parsed-argv is a much safer way to invoke another
program. You don't need the shell to interpret the boundaries of command line
arguments, it invites trouble.

~~~
archi42
Doesn't execve() end in the same syscall as system()? Not as comfortable for
exploitation, but maybe still feasible? (Embedded systems I take a look at
usually don't have an OS with either of the two, so I honestly don't know how
the implementations look like).

~~~
asveikau
system() basically does a fork and an execve of sh -c 'arg passed to
system()', then waitpid to block on the result. execve is a syscall, system is
a libc function.

------
userbinator
The GIF format is over 30 years old. One would think that decoding them was a
problem solved long ago, and that decoding libraries would have all the bugs
found and fixed by now.

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.)

------
big_chungus
The demo clip has been disabled by google drive due to too many plays. Does
anyone have a copy/backup he'd be willing to share?

------
jimbo1qaz
I recall someone once designed an image format which stored (image width - 1),
and added 1 to the image dimensions while decoding. That eliminates an edge
case, but no clue what would happen with MAX_INT image size...

------
gosuri
_To prevent falling victim to this attack, it is highly recommended that all
WhatsApp users update the app to the latest version._

Do I have to do this myself? Doesn't update itself?

~~~
GrayShade
It usually does, but you can disable updates in Play Store.

> The exploit works well until WhatsApp version 2.19.230. The vulnerability is
> official patched in WhatsApp version 2.19.244

------
nielsbot
This is a very easy to follow explanation, even for those not familiar with
these types of attacks. (me)

------
notzuck
Assuming version numbers of acceptable, he should have sold this to Zerodium.

------
billpg
Exactly how terrified should I be right now?

------
mruts
How does one go about finding stuff like this? Also how much time did it take
to research and develop the full exploit? Sounds like a lot of work,
especially if you’re not getting paid.

~~~
archi42
I think the author can answer this better, but I'd guess it's mixture of
hobby, curiosity and luck (or rather good intuition where $stuff breaks).
[edit] Can be anything from a Saturday if it is the first thing you poke into
(and easily grasp the call/stack structure) to a few weekends trying different
vectors (or figuring out how to layout your fake memory).[/edit]

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]

------
fourier_mode
> 24 minute read

A simple (number_of_words)/(200WPM) isn't a great metric when words from code
snippets are also counted.

~~~
hk__2
What would you suggest instead? We still read words from code snippets.

~~~
fourier_mode
According to that metric you spend 1 minute to read this --

    
    
        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

