brazdil / dexter

1 stars 1 forks source link

How does FILL_ARRAY_DATA work? Is this a relevant piece of code? #16

Closed brazdil closed 11 years ago

brazdil commented 11 years ago

There seems to be very little documentation about the actual semantics of instructions. I'm currently looking at the FILL_ARRAY_DATA instruction which stores constants into primitive arrays. However, I'm having trouble figuring out what happens when there are more elements provided to the instruction than what is the length of the array.

I've managed to run a short piece of code that does this and it (unsurprisingly) throws an ArrayBounds exception. The question is whether the data in the lower indexes were overwritten (i.e. it works like a loop; seems more likely) or not (i.e. all-or-nothing semantics), which affects the choice of instrumentation. I've written the following piece of code to decide which it is:

    # create array
    const/4 v0, 3
    new-array v0, v0, [I

    # store (potentially tainted) argument in it
    const/4 v1, 0
    aput p1, v0, v1

    # now overwrite with the constant data
    :try_start
    fill-array-data v0, :array_data
    :try_end
    .catchall {:try_start .. :try_end} :handler

    # retrieve and return
    aget v0, v0, v1
    return v0

    :handler

    # print the content of the array
    const/4 v1, 0
    aget v0, v0, v1
    invoke-static {v0}, Ljava/lang/Integer;->toString(I)Ljava/lang/String;
    move-result v1
    invoke-static {v1, v1}, Landroid/util/Log;->d(Ljava/lang/String;Ljava/lang/String;)I

    return v0

    :array_data
    .array-data 0x4
        0x00t 0x11t 0x22t 0x33t
        0xfft 0xeet 0xddt 0xcct
        0xfft 0xeet 0xddt 0xcct
        0xfft 0xeet 0xddt 0xcct
    .end array-data

It has an array of length three, stores a tainted integer inside and then calls FILL_ARRAY_DATA with four elements which throws an exception. When that happens, the previously tainted element is retrieved and printed in the Log, which should show whether the value has changed or not.

However, pushing this through Dexter fails with this error:

CODE DUMP:
0: const v0, #3
1: new-array v0, [v0], [I
2: const v1, #0
3: aput-int-float v3, {v0}[v1]
4: TRYSTART1
5: fill-array-data v0, <data>
6: TRYEND1
7: aget-int-float v0, {v0}[v1]
8: return v0
9: CATCHALL1
10: const v1, #0
11: aget-int-float v0, {v0}[v1]
12: invoke-static Ljava/lang/Integer;->toString(I)Ljava/lang/String; (v0)
13: move-result v1
14: invoke-static Landroid/util/Log;->d(Ljava/lang/String;Ljava/lang/String;)I (v1, v1)
15: return v0
Exception in thread "main" java.lang.Error: java.lang.AssertionError: Cannot find definer for v0 inside basic block 9-11
    at com.rx201.dx.translator.DexCodeAnalyzer.analyze(DexCodeAnalyzer.java:100)
    at uk.ac.cam.db538.dexter.transform.taint.TaintTransform.doFirst(TaintTransform.java:140)
    at uk.ac.cam.db538.dexter.transform.Transform.apply(Transform.java:77)
    at uk.ac.cam.db538.dexter.transform.Transform.apply(Transform.java:53)
    at uk.ac.cam.db538.dexter.MainConsole.main(MainConsole.java:65)
Caused by: java.lang.AssertionError: Cannot find definer for v0 inside basic block 9-11
    at com.rx201.dx.translator.DexCodeAnalyzer.livenessAnalysis(DexCodeAnalyzer.java:133)
    at com.rx201.dx.translator.DexCodeAnalyzer.analyze(DexCodeAnalyzer.java:87)
    ... 4 more

which seems to be related to issue #13, though I am not sure whether this is a relevant piece of code or not, as it uses registers defined before an exception was caught. I've been trying to run this directly using dalvikvm command on the device, but I seem to have trouble running anything at all...

brazdil commented 11 years ago

Finally!!! Managed to run the example and the answer is "all-or-nothing", i.e. if the array is too short, ArrayIndexOutOfBoundsException is thrown and nothing is overwritten.

brazdil commented 11 years ago

Okey, so the compilation error still needs fixing...

xurubin commented 11 years ago

Compilation error caused by not declaring FillArrayData as throwable. Fixed in 6bca81a.

brazdil commented 11 years ago

Hmm, this isn't really fixed yet. Recompilation of a FillArrayData instruction works only when it is not surrounded by a TRY block. In that case, DexInstructionTranslator.translate method takes the else if (successors.size() == 1) branch and everything is fine, but when the if (successors.size() > 1) branch is taken, translated instruction doesn't have any successors set. The reason is that the corresponding visit method does not generate them. I'm, however, not sure how this should be correctly handled, so I'm positing it here. What doesn't make sense to me is that the compiler's FillArrayDataInsn class returns empty catchers list.