-
Notifications
You must be signed in to change notification settings - Fork 138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tainted data not flowing through custom propagations #129
Comments
You are using For instance:
|
Thanks @arthaud! I have modified the propagation according to your comments. {
"model_generators" : [
{
"find": "methods",
"where": [
{
"constraint": "signature_pattern",
"pattern": "Lorg/json/JSONObject;\\.<init>.*"
}
],
"model": {
"propagation": [
{
"input": "Argument(1)",
"output": "Return"
}
]
},
"verbosity": 3
},
{
"find": "methods",
"where": [
{
"constraint": "signature_pattern",
"pattern": "Lorg/json/JSONObject;\\..*get.*"
}
],
"model": {
"propagation": [
{
"input": "Argument(0)",
"output": "Return"
}
]
},
"verbosity": 3
},
{
"find": "methods",
"where": [
{
"constraint": "signature_pattern",
"pattern": "Lorg/json/JSONObject;\\..*put.*"
}
],
"model": {
"propagation": [
{
"input": "Argument(2)",
"output": "Argument(0)"
}
]
},
"verbosity": 3
}
]
} But while running mariana trench I get the following log for source model
Any reason why this is happening? I am using the same model for the source as mentioned before. This is how the Tainted class looks like. package com.example.flowcheck;
public class Tainted {
public String source() {
return
"<some string here>";
}
public int sink(int mdc) {
Log.d("FlowCheck", "found: "+ mdc);
return 0;
}
} |
I'm really confused here. According to your first message, it seems like you have 3 model generator files, Source.json, Sink.json and Propagation.json. Then, your Another problem is that you forgot the
Unfortunately, Mariana Trench doesn't detect such typos which lead to a lot of confusion. We should probably improve this. |
fe18ac4 should prevent this error in the future. |
Thanks for the commit, should help a lot with debugging. I have fixed the typo in the source model and a few issues with the propagation. {
"model_generators" : [
{
"find": "methods",
"where": [
{
"constraint": "signature_pattern",
"pattern": "Lorg/json/JSONObject;\\.<init>.*"
}
],
"model": {
"propagation": [
{
"input": "Argument(0)",
"output": "Return"
}
]
},
"verbosity": 3
},
{
"find": "methods",
"where": [
{
"constraint": "signature_pattern",
"pattern": "Lorg/json/JSONObject;\\..*get.*"
}
],
"model": {
"propagation": [
{
"input": "Argument(0)",
"output": "Return"
}
]
},
"verbosity": 3
},
{
"find": "methods",
"where": [
{
"constraint": "signature_pattern",
"pattern": "Lorg/json/JSONObject;\\..*put.*"
}
],
"model": {
"propagation": [
{
"input": "Argument(2)",
"output": "Argument(0)"
}
]
},
"verbosity": 3
}
]
} This is how it looks now. But the propagation still does not work as expected since I don't get any result for the following sample target class that I am trying to analyze. I have commented the 3 places where I am trying to check for tainted data none of these sinks are getting tainted. public int GetMotionDetectConfig(int i, MotionDetectConfig motionDetectConfig) {
HashMap HashMap = new HashMap();
HashMap.put("cfgType", 1);
String jSONObject = new JSONObject(HashMap).toString();
Response response = new Response();
Tainted tt = new Tainted();
int sendCommand = 0;
response.resp = tt.source();
tt.sink(response.resp); // finds an issue here
Log.d("FlowCheck", "GET_MOTION_DETECT_CONFIG:" + response.resp);
if (sendCommand != 0 || TextUtils.isEmpty(response.resp)) {
return 1;
}
try {
JSONObject jSONObject2 = new JSONObject(response.resp);
tt.sink(jSONObject2); // Does not find an issue here
if (jSONObject2.isNull("ret")) {
return 1;
}
int i2 = jSONObject2.getInt("ret");
tt.sink(i2); // Does not find an issue here.
Log.d("FlowCheck", "ret = "+i2);
if (i2 == 0) {
if (!jSONObject2.isNull("channel")) {
motionDetectConfig.channel = jSONObject2.getInt("channel");
}
if (!jSONObject2.isNull("enable")) {
motionDetectConfig.enable = jSONObject2.getInt("enable");
}
if (!jSONObject2.isNull("linkage")) {
motionDetectConfig.linkage = jSONObject2.getInt("linkage");
}
if (!jSONObject2.isNull("snapInterval")) {
motionDetectConfig.snapInterval = jSONObject2.getInt("snapInterval");
}
if (!jSONObject2.isNull("triggerInterval")) {
motionDetectConfig.triggerInterval = jSONObject2.getInt("triggerInterval");
}
if (!jSONObject2.isNull("moveAlarmEnable")) {
motionDetectConfig.moveAlarmEnable = jSONObject2.getInt("moveAlarmEnable");
}
if (!jSONObject2.isNull("pirAlarmEnable")) {
motionDetectConfig.pirAlarmEnable = jSONObject2.getInt("pirAlarmEnable");
}
if (!jSONObject2.isNull("schedule")) {
JSONArray jSONArray = jSONObject2.getJSONArray("schedule");
motionDetectConfig.schedule = new long[jSONArray.length()];
int i3 = 0;
while (true) {
long[] jArr = motionDetectConfig.schedule;
if (i3 >= jArr.length) {
break;
}
jArr[i3] = jSONArray.getLong(i3);
i3++;
}
}
if (!jSONObject2.isNull("sensitivity")) {
motionDetectConfig.sensitivity = jSONObject2.getInt("sensitivity");
}
if (!jSONObject2.isNull("area")) {
JSONArray json_obj = jSONObject2.getJSONArray("area");
if (json_obj.length() > 3) {
motionDetectConfig.area = new int[json_obj.length()];
for (int idx = 0; idx < motionDetectConfig.schedule.length; idx++) {
tt.sink(json_obj.getInt(idx)); // Does not find an issue here
motionDetectConfig.area[idx] = json_obj.getInt(idx);
}
}
}
}
return i2;
} catch (JSONException e) {
e.printStackTrace();
return 1;
}
} Tainted.java public class Tainted {
public String source() {
return
"<some string here>";
}
public int sink(int mdc) {
Log.d("FlowCheck", "found: "+ mdc);
return 0;
}
public int sink(String str) {
Log.d("FlowCheck", "found "+ str);
return 0;
}
public void sink(JSONObject jo){
Log.d("FlowCheck", "found "+ jo);
}
} mariana-trench logs:
|
Could you try using |
Attached the logs for the function. |
I have attached the apk here to make things easier. |
Thanks. I think you made a mistake in the models. For
The constructor is actually a non-static method, hence |
Yes, I have tried both Argument(0) and Argument(1) the result remains the same where only this is being detected but the rest of the flows are not detected.
I have attached mariana trench logs with the propagation input as Argument(1). |
oh my bad, I think you want a propagation from
|
This is because we analyze the Android bytecode. When you write:
The constructor does not really "return" anything, it just updates the implicit
Probably because you did not provide the code for |
The android jars that are shipped with the SDK just have this for a lot of similar functions public JSONArray put(int index, double value) throws JSONException {
throw new RuntimeException("Stub!");
} Is this what causes the issue of mariana-trench not propagating the taint properly? |
Yes this could be a problem. We would have to use a model generator to cover those. |
I am trying to verify custom propagations via a JSONObject in an APK, The source and sink are detected accurately but mt is not detecting the expected flow. Am I configuring something wrong here?
Sink:
Source:
Propagation:
The idea here is to taint return value of get* and taint the object itself for put* in org.json.JSONObject's methods.
rules.json
default-model-generator.json
The text was updated successfully, but these errors were encountered: