maaaaz / androwarn

Yet another static code analyzer for malicious Android applications
GNU Lesser General Public License v3.0
473 stars 159 forks source link

bugs found #8

Open ririhedou opened 9 years ago

ririhedou commented 9 years ago

I have tried this tool by analyzing application writen by myself. It has a sensitive smsMessage sending API to read content from user 's input (editText) and then send it to a constant number.

But from the report we can see: 1 it seems contradictary from my source codes becasue it shows sending "1" to "1".

Then I output the telephone-service part:

Class 'Landroid/telephony/SmsManager;' - Method 'sendTextMessage' - Register state before call [{'0': 'Landroid/telephony/SmsManager;->getDefault()Landroid/telephony/SmsManager;'}, {'1': '1'}, {'2': 'button pressed'}, {'1': '1'}, {'4': 'v2'}, {'5': 'v2'}]

also why {'4': 'v2'} exists is becasue we have a instruction "move-object v4, v2", but it seems match_current_instruction() method does not split it correctly and regards "v2" as a constant value not a register.

Then I found that the bug contained in the backtrace_registers_before_call method in core.py.
It seems that there is a misunderstanding about how opcode "move-result" works. After I modified the code, Here is new results output from the same apk I tested:

new1

I am currently working on this and will update the codes after I finishing it.

maaaaz commented 9 years ago

Hello riridehou,

Thanks for you feedback. I'm really looking forward to your patches, my code is for sure not bug free ! :)

ririhedou commented 9 years ago

I have some questions on the core part about the data flow analysis, just a little confused.

1 it seems that androwarn only consider all the data dependence in one method, but how about the dependence happened by inter-method communication.

Assume that sendtextMessage() function have 5 parameters from v1- v5, what if the third parameter v3's value is based on other method. e.x there is a call relations between b -> a(p0,p1). And this sensitive API sendtextMessage() is contained in method b. How can we deal with this case? the VManalysis.tainted_packages.search_methods in androguard can provide some functionality to locate which method will call a, but I found that it have something confuses me:

if there are one method with different parameters, we will get the smali code looks like:

.method public static a(Ljava/lang/String;Ljava/lang/String;)V // smali code

.method public static a(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Z // smali code

and the sendtextMessage() only happened in " .method public static a(Ljava/lang/String;Ljava/lang/String;)V " method

And in the package it have different calls like: b1 calls a with 2 parameters b2 calls a with 3 parameters

but VManalysis.tainted_packages.search_methods in androguard seems cannot distinguish the 2 different situations. And I think this is a little hard to accomplish it in static analysis.

2. in the regular pattern in "match_current_instruction" in core method. why androwarn does not consider "iget" "aget" instruction? also the "invoke" may have 3 or more registers.

3. if we get something like in the smali code:

invoke-static {v15}, Lcom/geinimi/Ad;->c(I)Ljava/lang/String; move-result-object v15

where "Lcom/geinimi/Ad;->c(I)Ljava/lang/String" means a will call c and store the return value in register "v15". In the androwarn, it will get something like : {'15': Lcom/geinimi/c/m;->a(I)Ljava/lang/String;}, but "Lcom/geinimi/c/m;->a(I)Ljava/lang/String;" is a method is the package and not be finally determined yet.

I try to finish them but found that there are so many exceptions there. :)