michalbednarski/ReparcelBug2

Re: One could construct data that will cause at end seek to final position

M3hh opened this issue · 4 comments

M3hh commented

Hello,

Very impressive find, can I ask one question Re: "although I think in many cases one could construct data that will cause at end seek to final position so this one isn't strong mitigation"

Can you elaborate on that a bit more?
Did you mean there are other parcelable classes can be used as the gadget to put in the PoC Parcel, that would do setPos when deserializing?

Thanks!

Advancing position within Parcel by controlled amount happens naturally at many points, for example in readString or createIntArray. For example the scheduleReceiver has String and Bundle arguments that are not relevant for exploit execution and exploit was providing values for them, relying on fact that remaining data will be ignored. If there was enforceNoDataAvail() check after reading these arguments I'd have to write length of string for in String data argument and terminate injected data at that point, so that string after reading will consist of leftover data

M3hh commented

If I am understanding correctly, for scheduleReceiver, the attacker would only be able to control the Intent object, right? As that is invoked by system_server, using the intent object received from the attacker's process as the first parameter. Meaning that you would need to construct "fake" ActivityInfo, compatInfo, ... within the Intent Object; followed by the real ActivityInfo, compatInfo, ... params, appended by system_server

Hence in this case, in the AIDL checker, it should always have remaining data in the Parcel, and hence failing the enforceNoDataAvail check, am I correct?

I do take the point that E.g. readString could be used to advance the pointers by giving it the calculated length; so that sounds like you would need to look for situations where the AIDL call takes E.g. (..., Intent, String) as the parameters, where it has to end with E.g. readString, and takes Intent before that. Am I understanding it correctly?

Yep, basically this exploit could be adapted so scheduleReceiver args are written/read as:

Written asRead as
1Beginning of IntentIntent intent
2Raw data in Bundle inside Intent
3ActivityInfo info
4CompatibilityInfo compatInfo
5int resultCode
6String data (length)
7Ending of IntentString data (text)
8ActivityInfo info
9CompatibilityInfo compatInfo
10int resultCode
11String data (length and text
(or just length=-1 if null))
12Bundle extras
13boolean sync
14int sendingUser
15int processState

Advance with controlled size is pretty common, aside of String data here there is readString8 inside ActivityInfo, Bundle (extras argument) has length

If exact length of remaining data is not known in advance the String data argument could be set (through initialData argument of sendOrderedBroadcast()) to the String with pattern of decreasing lengths, so each landing dataPosition inside it would lead to advancing dataPosition to end of that string (positions are always aligned to 4 bytes and length is also written as 4 bytes)

Also many Parcel read methods fail silently (don't throw) when they are at end of Parcel, for example readInt will return 0 and not change dataPosition if dataAvail==0

M3hh commented

Very cool thanks!