Skip to content

Commit 2ab7a55

Browse files
atorralbaatorralba
committed
Improve intermediate flow to add more potential sources
1 parent 28369d1 commit 2ab7a55

File tree

3 files changed

+131
-13
lines changed

3 files changed

+131
-13
lines changed

‎java/ql/src/semmle/code/java/security/AndroidIntentRedirection.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ private class DefaultIntentRedirectionSanitizer extends IntentRedirectionSanitiz
7070
DefaultIntentRedirectionSanitizer() {
7171
exists(MethodAccess ma, Method m |
7272
ma.getMethod() = m and
73-
m.hasQualifiedName("android.content", "ComponentName", ["getPackageName", "getClassName"]) and
73+
m.getDeclaringType() instanceof TypeComponentName and
74+
m.hasName(["getPackageName", "getClassName"]) and
7475
ma.getBasicBlock().(ConditionBlock).controls(this.asExpr().getBasicBlock(), true)
7576
)
7677
}

‎java/ql/src/semmle/code/java/security/AndroidIntentRedirectionQuery.qll

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
import java
44
import semmle.code.java.dataflow.FlowSources
55
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.dataflow.TaintTracking2
67
import semmle.code.java.security.AndroidIntentRedirection
78

89
/**
9-
* A taint tracking configuration for user-provided Intents being used to start Android components.
10+
* A taint tracking configuration for tainted Intents being used to start Android components.
1011
*/
1112
class IntentRedirectionConfiguration extends TaintTracking::Configuration {
1213
IntentRedirectionConfiguration() { this = "IntentRedirectionConfiguration" }
@@ -24,34 +25,68 @@ class IntentRedirectionConfiguration extends TaintTracking::Configuration {
2425
}
2526
}
2627

27-
/** The method `getParcelableExtra` called on a tainted `Intent`. */
28+
/** An expression modifying an `Intent` component with tainted data. */
2829
private class IntentRedirectionSource extends DataFlow::Node {
2930
IntentRedirectionSource() {
30-
exists(GetParcelableExtra ma | this.asExpr() = ma.getQualifier()) and
31-
exists(IntentToGetParcelableExtraConf conf | conf.hasFlowTo(this))
31+
changesIntentComponent(this.asExpr()) and
32+
exists(TaintedIntentComponentConf conf | conf.hasFlowTo(this))
3233
}
3334
}
3435

3536
/**
36-
* Data flow from a remote intent to the qualifier of a `getParcelableExtra` call.
37+
* A taint tracking configuration for tainted data flowing to an `Intent`'s component.
3738
*/
38-
private class IntentToGetParcelableExtraConf extends DataFlow2::Configuration {
39-
IntentToGetParcelableExtraConf() { this = "IntentToGetParcelableExtraConf" }
39+
private class TaintedIntentComponentConf extends TaintTracking2::Configuration {
40+
TaintedIntentComponentConf() { this = "TaintedIntentComponentConf" }
4041

4142
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
4243

43-
override predicate isSink(DataFlow::Node sink) {
44-
exists(GetParcelableExtra ma | sink.asExpr() = ma.getQualifier())
45-
}
44+
override predicate isSink(DataFlow::Node sink) { changesIntentComponent(sink.asExpr()) }
45+
}
46+
47+
/** Holds if `expr` modifies the component of an `Intent`. */
48+
private predicate changesIntentComponent(Expr expr) {
49+
any(IntentGetParcelableExtra igpe).getQualifier() = expr or
50+
any(IntentSetComponent isc).getSink() = expr
4651
}
4752

4853
/** A call to the method `Intent.getParcelableExtra`. */
49-
private class GetParcelableExtra extends MethodAccess {
50-
GetParcelableExtra() {
54+
private class IntentGetParcelableExtra extends MethodAccess {
55+
IntentGetParcelableExtra() {
5156
exists(Method m |
5257
this.getMethod() = m and
5358
m.getDeclaringType() instanceof TypeIntent and
5459
m.hasName("getParcelableExtra")
5560
)
5661
}
5762
}
63+
64+
/** A call to a method that changes the component of an `Intent`. */
65+
private class IntentSetComponent extends MethodAccess {
66+
int sinkArg;
67+
68+
IntentSetComponent() {
69+
exists(Method m |
70+
this.getMethod() = m and
71+
m.getDeclaringType() instanceof TypeIntent
72+
|
73+
m.hasName("setClass") and
74+
sinkArg = 1
75+
or
76+
m.hasName("setClassName") and
77+
exists(Parameter p |
78+
p = m.getAParameter() and
79+
p.getType() instanceof TypeString and
80+
sinkArg = p.getPosition()
81+
)
82+
or
83+
m.hasName("setComponent") and
84+
sinkArg = 0
85+
or
86+
m.hasName("setPackage") and
87+
sinkArg = 0
88+
)
89+
}
90+
91+
Expr getSink() { result = this.getArgument(sinkArg) }
92+
}

‎java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.java

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.example.app;
22

33
import android.app.Activity;
4+
import android.content.ComponentName;
45
import android.content.Context;
56
import android.content.Intent;
67
import android.os.Bundle;
@@ -54,5 +55,86 @@ public void onCreate(Bundle savedInstanceState) {
5455
sendStickyOrderedBroadcast(intent, null, null, 0, null, null); // $ hasAndroidIntentRedirection
5556
sendStickyOrderedBroadcastAsUser(intent, null, null, null, 0, null, null); // $ hasAndroidIntentRedirection
5657
// @formatter:on
58+
59+
try {
60+
{
61+
Intent fwdIntent = new Intent();
62+
fwdIntent.setClassName((Context) null, (String) intent.getExtra("className"));
63+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
64+
}
65+
{
66+
Intent fwdIntent = new Intent();
67+
fwdIntent.setClassName((String) intent.getExtra("packageName"), null);
68+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
69+
}
70+
{
71+
Intent fwdIntent = new Intent();
72+
fwdIntent.setClassName((String) intent.getExtra("packageName"),
73+
(String) intent.getExtra("className"));
74+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
75+
}
76+
{
77+
Intent fwdIntent = new Intent();
78+
fwdIntent.setClass(null, Class.forName((String) intent.getExtra("className")));
79+
// needs taint step for Class.forName
80+
startActivity(fwdIntent); // $ MISSING: $hasAndroidIntentRedirection
81+
}
82+
{
83+
Intent fwdIntent = new Intent();
84+
fwdIntent.setPackage((String) intent.getExtra("packageName"));
85+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
86+
}
87+
{
88+
Intent fwdIntent = new Intent();
89+
ComponentName component =
90+
new ComponentName((String) intent.getExtra("packageName"), null);
91+
fwdIntent.setComponent(component);
92+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
93+
}
94+
{
95+
Intent fwdIntent = new Intent();
96+
ComponentName component =
97+
new ComponentName("", (String) intent.getExtra("className"));
98+
fwdIntent.setComponent(component);
99+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
100+
}
101+
{
102+
Intent fwdIntent = new Intent();
103+
ComponentName component =
104+
new ComponentName((Context) null, (String) intent.getExtra("className"));
105+
fwdIntent.setComponent(component);
106+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
107+
}
108+
{
109+
Intent fwdIntent = new Intent();
110+
ComponentName component = new ComponentName((Context) null,
111+
Class.forName((String) intent.getExtra("className")));
112+
fwdIntent.setComponent(component);
113+
// needs taint step for Class.forName
114+
startActivity(fwdIntent); // $ MISSING: $hasAndroidIntentRedirection
115+
}
116+
{
117+
Intent fwdIntent = new Intent();
118+
ComponentName component =
119+
ComponentName.createRelative("", (String) intent.getExtra("className"));
120+
fwdIntent.setComponent(component);
121+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
122+
}
123+
{
124+
Intent fwdIntent = new Intent();
125+
ComponentName component =
126+
ComponentName.createRelative((String) intent.getExtra("packageName"), "");
127+
fwdIntent.setComponent(component);
128+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
129+
}
130+
{
131+
Intent fwdIntent = new Intent();
132+
ComponentName component = ComponentName.createRelative((Context) null,
133+
(String) intent.getExtra("className"));
134+
fwdIntent.setComponent(component);
135+
startActivity(fwdIntent); // $ hasAndroidIntentRedirection
136+
}
137+
} catch (Exception e) {
138+
}
57139
}
58140
}

0 commit comments

Comments
 (0)