cipherpunk / google-security-research

Automatically exported from code.google.com/p/google-security-research
0 stars 0 forks source link

Adobe Flash: Type Confusion in IExternalizable.writeExternal When Performing Local Serialization #547

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
If IExternalizable.writeExternal is overridden with a value that is not a 
function, Flash assumes it is a function even though it is not one. This leads 
to execution of a 'method' outside of the ActionScript object's ActionScript 
vtable, leading to memory corruption.

A sample swf is attached. ActionScript code is also attached, but it does not 
compile to the needed to swf. To get the PoC, decompress the swf using flasm -x 
myswf, and then search for "triteExternal" and change it to "writeExternal".

This bug is subject to a 90 day disclosure deadline. If 90 days elapse
without a broadly available patch, then the bug report will automatically
become visible to the public.

Original issue reported on code.google.com by natashe...@google.com on 30 Sep 2015 at 1:31

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by natashe...@google.com on 15 Oct 2015 at 10:34

GoogleCodeExporter commented 8 years ago
Adding some more information about this bug, since it's in the open source 
components of Flash, and is in the wild.

This bug is in the AVM serializer 
(http://hg.mozilla.org/tamarin-redux/file/5571cf86fc68/core/AvmSerializer.cpp), 
and is type confusion when calling the method writeExternal, which is 
implemented when a class extends IExternalizable 
(http://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/flash/utils/
IExternalizable.html). The method is resolved on line 1437 of AvmSerializer.cpp 
by calling toplevel->getBinding, which does not guarantee that the binding is a 
method binding. It then gets cast to a method on line 773 and called, which is 
type confusion.

One challenge with the bug is actually creating a SWF which can hit this code, 
as usually overriding a defined method will lead to an illegal override 
exception. The 0-day author did this differently than I did. The code where all 
class properties (methods, internal classes, variables, etc.) are resolved is 
in http://hg.mozilla.org/tamarin-redux/file/5571cf86fc68/core/Traits.cpp. You 
can see on line 813 that a check that no two properties of a class have the 
same name is commented out due to some legitimate SWFs doing that. This means 
that a SWF can have a variable with the same name as a method (overriding a 
method with less restrictive method is still illegal), which is how my PoC 
overrode the method. The 0-day did something slightly different, it put the 
redefinition of writeExternal in a different public namespace than the original 
definition of writeExternal. This has the benefit that the ActionScript will 
compile and hit the bug without modification. 

Original comment by natashe...@google.com on 16 Oct 2015 at 6:03

GoogleCodeExporter commented 8 years ago

Original comment by natashe...@google.com on 16 Oct 2015 at 6:08

GoogleCodeExporter commented 8 years ago
Thanks for the great explanation, however i really cannot understand how this 
is leading to a memory corruption since you're just calling a method outside 
vtable, i know this should not be possibile anyway i would like to better 
understand where memory corruption happen.

best regards,

Daniele Linguaglossa

Original comment by danielel...@gmail.com on 21 Oct 2015 at 9:09

GoogleCodeExporter commented 8 years ago
"Memory corruption : an unintentional modification of a memory location due to 
an error in a computer program". In this case the attacker can control the 
memory location that the AVM assumes is within the vtable (due to the type 
confusion described above). In the case of the POC this is not a valid memory 
address so it causes a crash at the mov instruction before the dynamic call.  

Original comment by athmi...@gmail.com on 27 Oct 2015 at 2:45

GoogleCodeExporter commented 8 years ago
Hello Natashenka, 

Is it possible to get your insight on the following:

In the class that overwrites the writeExternal() method, why is the test() 
method required to trigger this vulnerability? When I remove this method (which 
seems to be doing nothing), it looks like the programs crashes due to a null 
pointer dereference instead. From my understanding, the program has a pointer 
to the vtable and calls a function @ vtable pointer + certain offset (in the 
normal case this is a pointer to the writeExternal function, in the POC case 
this is some garbage value). When I remove the test() method, the program seems 
to have calculates the vtable pointer value wrong (pointer value ~=0) and seems 
to die trying to dereference a pointer value of ~=0. This tells me that some 
check somewhere else went wrong and the program is dying due to a null pointer 
dereference. Would you have any insight as to what this check could be?  

Original comment by athmi...@gmail.com on 28 Oct 2015 at 9:43

GoogleCodeExporter commented 8 years ago
The extra method changes the size of the method table, and as a result where it 
is allocated on the heap and/or what surrounds it. My guess is that your null 
pointer crash happens because removing the test method causes the out-of-bounds 
method index to now point to something that is zero in memory, when previously 
it was a different value.

Original comment by natashe...@google.com on 2 Nov 2015 at 8:30

GoogleCodeExporter commented 8 years ago
That makes sense! Thank you for the information/clarification

Original comment by athmi...@gmail.com on 3 Nov 2015 at 7:24