Skip to content

Commit f4704f1

Browse files
atorralbaatorralba
authored
Merge pull request #6397 from atorralba/atorralba/android-intent-redirect-query
Java: Create new Android Intent Redirection query
2 parents e9b1146 + fd92c4e commit f4704f1

28 files changed

+1430
-1716
lines changed

‎java/change-notes/2021-08-02-android-intent-redirect-query.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
lgtm,codescanning
2+
* A new query "Android Intent redirection" (`java/android/intent-redirection`) has been added.
3+
This query finds exported Android components using received Intents to start other components,
4+
which can provide access to internal components of the application or cause other unintended
5+
effects.

‎java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ private module Frameworks {
107107
private import semmle.code.java.frameworks.spring.SpringBeans
108108
private import semmle.code.java.frameworks.spring.SpringWebMultipart
109109
private import semmle.code.java.frameworks.spring.SpringWebUtil
110+
private import semmle.code.java.security.AndroidIntentRedirection
110111
private import semmle.code.java.security.ResponseSplitting
111112
private import semmle.code.java.security.InformationLeak
112113
private import semmle.code.java.security.GroovyInjection

‎java/ql/lib/semmle/code/java/frameworks/android/Intent.qll

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ class TypeIntent extends Class {
1010
TypeIntent() { this.hasQualifiedName("android.content", "Intent") }
1111
}
1212

13+
/** The class `android.content.ComponentName`. */
14+
class TypeComponentName extends Class {
15+
TypeComponentName() { this.hasQualifiedName("android.content", "ComponentName") }
16+
}
17+
1318
/**
1419
* The class `android.app.Activity`.
1520
*/
@@ -293,3 +298,34 @@ private class IntentBundleFlowSteps extends SummaryModelCsv {
293298
]
294299
}
295300
}
301+
302+
private class IntentComponentTaintSteps extends SummaryModelCsv {
303+
override predicate row(string s) {
304+
s =
305+
[
306+
"android.content;Intent;true;Intent;(Intent);;Argument[0];Argument[-1];taint",
307+
"android.content;Intent;true;Intent;(Context,Class);;Argument[1];Argument[-1];taint",
308+
"android.content;Intent;true;Intent;(String,Uri,Context,Class);;Argument[3];Argument[-1];taint",
309+
"android.content;Intent;true;getIntent;(String);;Argument[0];ReturnValue;taint",
310+
"android.content;Intent;true;getIntentOld;(String);;Argument[0];ReturnValue;taint",
311+
"android.content;Intent;true;parseUri;(String,int);;Argument[0];ReturnValue;taint",
312+
"android.content;Intent;true;setPackage;;;Argument[0];Argument[-1];taint",
313+
"android.content;Intent;true;setClass;;;Argument[1];Argument[-1];taint",
314+
"android.content;Intent;true;setClassName;(Context,String);;Argument[1];Argument[-1];taint",
315+
"android.content;Intent;true;setClassName;(String,String);;Argument[0..1];Argument[-1];taint",
316+
"android.content;Intent;true;setComponent;;;Argument[0];Argument[-1];taint",
317+
"android.content;ComponentName;false;ComponentName;(String,String);;Argument[0..1];Argument[-1];taint",
318+
"android.content;ComponentName;false;ComponentName;(Context,String);;Argument[1];Argument[-1];taint",
319+
"android.content;ComponentName;false;ComponentName;(Context,Class);;Argument[1];Argument[-1];taint",
320+
"android.content;ComponentName;false;ComponentName;(Parcel);;Argument[0];Argument[-1];taint",
321+
"android.content;ComponentName;false;createRelative;(String,String);;Argument[0..1];ReturnValue;taint",
322+
"android.content;ComponentName;false;createRelative;(Context,String);;Argument[1];ReturnValue;taint",
323+
"android.content;ComponentName;false;flattenToShortString;;;Argument[-1];ReturnValue;taint",
324+
"android.content;ComponentName;false;flattenToString;;;Argument[-1];ReturnValue;taint",
325+
"android.content;ComponentName;false;getClassName;;;Argument[-1];ReturnValue;taint",
326+
"android.content;ComponentName;false;getPackageName;;;Argument[-1];ReturnValue;taint",
327+
"android.content;ComponentName;false;getShortClassName;;;Argument[-1];ReturnValue;taint",
328+
"android.content;ComponentName;false;unflattenFromString;;;Argument[0];ReturnValue;taint"
329+
]
330+
}
331+
}

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

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/** Provides classes to reason about Android Intent redirect vulnerabilities. */
2+
3+
import java
4+
private import semmle.code.java.controlflow.Guards
5+
private import semmle.code.java.dataflow.DataFlow
6+
private import semmle.code.java.dataflow.ExternalFlow
7+
private import semmle.code.java.frameworks.android.Intent
8+
9+
/**
10+
* A sink for Intent redirection vulnerabilities in Android,
11+
* that is, method calls that start Android components (like activities or services).
12+
*/
13+
abstract class IntentRedirectionSink extends DataFlow::Node { }
14+
15+
/** A sanitizer for data used to start an Android component. */
16+
abstract class IntentRedirectionSanitizer extends DataFlow::Node { }
17+
18+
/**
19+
* A unit class for adding additional taint steps.
20+
*
21+
* Extend this class to add additional taint steps that should apply to `IntentRedirectionConfiguration`.
22+
*/
23+
class IntentRedirectionAdditionalTaintStep extends Unit {
24+
/**
25+
* Holds if the step from `node1` to `node2` should be considered a taint
26+
* step for the `IntentRedirectionConfiguration` configuration.
27+
*/
28+
abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
29+
}
30+
31+
private class DefaultIntentRedirectionSinkModel extends SinkModelCsv {
32+
override predicate row(string row) {
33+
row =
34+
[
35+
"android.app;Activity;true;bindService;;;Argument[0];intent-start",
36+
"android.app;Activity;true;bindServiceAsUser;;;Argument[0];intent-start",
37+
"android.app;Activity;true;startActivityAsCaller;;;Argument[0];intent-start",
38+
"android.app;Activity;true;startActivityForResult;(Intent,int);;Argument[0];intent-start",
39+
"android.app;Activity;true;startActivityForResult;(Intent,int,Bundle);;Argument[0];intent-start",
40+
"android.app;Activity;true;startActivityForResult;(String,Intent,int,Bundle);;Argument[1];intent-start",
41+
"android.app;Activity;true;startActivityForResultAsUser;;;Argument[0];intent-start",
42+
"android.content;Context;true;startActivities;;;Argument[0];intent-start",
43+
"android.content;Context;true;startActivity;;;Argument[0];intent-start",
44+
"android.content;Context;true;startActivityAsUser;;;Argument[0];intent-start",
45+
"android.content;Context;true;startActivityFromChild;;;Argument[1];intent-start",
46+
"android.content;Context;true;startActivityFromFragment;;;Argument[1];intent-start",
47+
"android.content;Context;true;startActivityIfNeeded;;;Argument[0];intent-start",
48+
"android.content;Context;true;startForegroundService;;;Argument[0];intent-start",
49+
"android.content;Context;true;startService;;;Argument[0];intent-start",
50+
"android.content;Context;true;startServiceAsUser;;;Argument[0];intent-start",
51+
"android.content;Context;true;sendBroadcast;;;Argument[0];intent-start",
52+
"android.content;Context;true;sendBroadcastAsUser;;;Argument[0];intent-start",
53+
"android.content;Context;true;sendBroadcastWithMultiplePermissions;;;Argument[0];intent-start",
54+
"android.content;Context;true;sendStickyBroadcast;;;Argument[0];intent-start",
55+
"android.content;Context;true;sendStickyBroadcastAsUser;;;Argument[0];intent-start",
56+
"android.content;Context;true;sendStickyOrderedBroadcast;;;Argument[0];intent-start",
57+
"android.content;Context;true;sendStickyOrderedBroadcastAsUser;;;Argument[0];intent-start"
58+
]
59+
}
60+
}
61+
62+
/** Default sink for Intent redirection vulnerabilities. */
63+
private class DefaultIntentRedirectionSink extends IntentRedirectionSink {
64+
DefaultIntentRedirectionSink() { sinkNode(this, "intent-start") }
65+
}
66+
67+
/**
68+
* A default sanitizer for nodes dominated by calls to `ComponentName.getPackageName`
69+
* or `ComponentName.getClassName`. These are used to check whether the origin or destination
70+
* components are trusted.
71+
*/
72+
private class DefaultIntentRedirectionSanitizer extends IntentRedirectionSanitizer {
73+
DefaultIntentRedirectionSanitizer() {
74+
exists(MethodAccess ma, Method m, Guard g, boolean branch |
75+
ma.getMethod() = m and
76+
m.getDeclaringType() instanceof TypeComponentName and
77+
m.hasName(["getPackageName", "getClassName"]) and
78+
g.isEquality(ma, _, branch) and
79+
g.controls(this.asExpr().getBasicBlock(), branch)
80+
)
81+
}
82+
}

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

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/** Provides taint tracking configurations to be used in Android Intent redirection queries. */
2+
3+
import java
4+
import semmle.code.java.dataflow.FlowSources
5+
import semmle.code.java.dataflow.DataFlow3
6+
import semmle.code.java.dataflow.TaintTracking
7+
import semmle.code.java.dataflow.TaintTracking2
8+
import semmle.code.java.security.AndroidIntentRedirection
9+
10+
/**
11+
* A taint tracking configuration for tainted Intents being used to start Android components.
12+
*/
13+
class IntentRedirectionConfiguration extends TaintTracking::Configuration {
14+
IntentRedirectionConfiguration() { this = "IntentRedirectionConfiguration" }
15+
16+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
17+
18+
override predicate isSink(DataFlow::Node sink) { sink instanceof IntentRedirectionSink }
19+
20+
override predicate isSanitizer(DataFlow::Node sanitizer) {
21+
sanitizer instanceof IntentRedirectionSanitizer
22+
}
23+
24+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
25+
any(IntentRedirectionAdditionalTaintStep c).step(node1, node2)
26+
}
27+
}
28+
29+
/**
30+
* A sanitizer for sinks that receive the original incoming Intent,
31+
* since its component cannot be arbitrarily set.
32+
*/
33+
private class OriginalIntentSanitizer extends IntentRedirectionSanitizer {
34+
OriginalIntentSanitizer() { any(SameIntentBeingRelaunchedConfiguration c).hasFlowTo(this) }
35+
}
36+
37+
/**
38+
* Data flow configuration used to discard incoming Intents
39+
* flowing directly to sinks that start Android components.
40+
*/
41+
private class SameIntentBeingRelaunchedConfiguration extends DataFlow3::Configuration {
42+
SameIntentBeingRelaunchedConfiguration() { this = "SameIntentBeingRelaunchedConfiguration" }
43+
44+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
45+
46+
override predicate isSink(DataFlow::Node sink) { sink instanceof IntentRedirectionSink }
47+
48+
override predicate isBarrier(DataFlow::Node barrier) {
49+
// Don't discard the Intent if its original component is tainted
50+
barrier instanceof IntentWithTaintedComponent
51+
}
52+
53+
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
54+
// Intents being built with the copy constructor from the original Intent are discarded too
55+
exists(ClassInstanceExpr cie |
56+
cie.getConstructedType() instanceof TypeIntent and
57+
node1.asExpr() = cie.getArgument(0) and
58+
node1.asExpr().getType() instanceof TypeIntent and
59+
node2.asExpr() = cie
60+
)
61+
}
62+
}
63+
64+
/** An `Intent` with a tainted component. */
65+
private class IntentWithTaintedComponent extends DataFlow::Node {
66+
IntentWithTaintedComponent() {
67+
exists(IntentSetComponent setExpr, TaintedIntentComponentConf conf |
68+
setExpr.getQualifier() = this.asExpr() and
69+
conf.hasFlowTo(DataFlow::exprNode(setExpr.getSink()))
70+
)
71+
}
72+
}
73+
74+
/**
75+
* A taint tracking configuration for tainted data flowing to an `Intent`'s component.
76+
*/
77+
private class TaintedIntentComponentConf extends TaintTracking2::Configuration {
78+
TaintedIntentComponentConf() { this = "TaintedIntentComponentConf" }
79+
80+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
81+
82+
override predicate isSink(DataFlow::Node sink) {
83+
any(IntentSetComponent setComponent).getSink() = sink.asExpr()
84+
}
85+
}
86+
87+
/** A call to a method that changes the component of an `Intent`. */
88+
private class IntentSetComponent extends MethodAccess {
89+
int sinkArg;
90+
91+
IntentSetComponent() {
92+
exists(Method m |
93+
this.getMethod() = m and
94+
m.getDeclaringType() instanceof TypeIntent
95+
|
96+
m.hasName("setClass") and
97+
sinkArg = 1
98+
or
99+
m.hasName("setClassName") and
100+
exists(Parameter p |
101+
p = m.getAParameter() and
102+
p.getType() instanceof TypeString and
103+
sinkArg = p.getPosition()
104+
)
105+
or
106+
m.hasName("setComponent") and
107+
sinkArg = 0
108+
or
109+
m.hasName("setPackage") and
110+
sinkArg = 0
111+
)
112+
}
113+
114+
Expr getSink() { result = this.getArgument(sinkArg) }
115+
}

‎java/ql/src/Security/CWE/CWE-940/AndroidIntentRedirection.qhelp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>An exported Android component that obtains a user-provided Intent and uses it to launch another component
5+
can be exploited to obtain access to private, unexported components of the same app or to launch other apps' components
6+
on behalf of the victim app.</p>
7+
</overview>
8+
<recommendation>
9+
<p>Do not export components that start other components from a user-provided Intent.
10+
They can be made private by setting the <code>android:exported</code> property to <code>false</code> in the app's Android Manifest.</p>
11+
<p>If this is not possible, restrict either which apps can send Intents to the affected component, or which components can be started from it.</p>
12+
</recommendation>
13+
<example>
14+
<p>The following snippet contains three examples.
15+
In the first example, an arbitrary component can be started from the externally provided <code>forward_intent</code> Intent.
16+
In the second example, the destination component of the Intent is first checked to make sure it is safe.
17+
In the third example, the component that created the Intent is first checked to make sure it comes from a trusted origin.</p>
18+
<sample src="AndroidIntentRedirectionSample.java" />
19+
</example>
20+
<references>
21+
<li>
22+
Google:
23+
<a href="https://support.google.com/faqs/answer/9267555?hl=en">Remediation for Intent Redirection Vulnerability</a>.
24+
</li>
25+
<li>
26+
OWASP Mobile Security Testing Guide:
27+
<a href="https://mobile-security.gitbook.io/mobile-security-testing-guide/android-testing-guide/0x05a-platform-overview#intents">Intents</a>.
28+
</li>
29+
<li>
30+
Android Developers:
31+
<a href="https://developer.android.com/guide/topics/manifest/activity-element#exported">The android:exported attribute</a>.
32+
</li>
33+
</references>
34+
</qhelp>

‎java/ql/src/Security/CWE/CWE-940/AndroidIntentRedirection.ql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Android Intent redirection
3+
* @description Starting Android components with user-provided Intents
4+
* can provide access to internal components of the application,
5+
* increasing the attack surface and potentially causing unintended effects.
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @security-severity 7.5
9+
* @precision high
10+
* @id java/android/intent-redirection
11+
* @tags security
12+
* external/cwe/cwe-926
13+
* external/cwe/cwe-940
14+
*/
15+
16+
import java
17+
import semmle.code.java.security.AndroidIntentRedirectionQuery
18+
import DataFlow::PathGraph
19+
20+
from DataFlow::PathNode source, DataFlow::PathNode sink, IntentRedirectionConfiguration conf
21+
where conf.hasFlowPath(source, sink)
22+
select sink.getNode(), source, sink,
23+
"Arbitrary Android activities or services can be started from $@.", source.getNode(),
24+
"this user input"

‎java/ql/src/Security/CWE/CWE-940/AndroidIntentRedirectionSample.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// BAD: A user-provided Intent is used to launch an arbitrary component
2+
Intent forwardIntent = (Intent) getIntent().getParcelableExtra("forward_intent");
3+
startActivity(forwardIntent);
4+
5+
// GOOD: The destination component is checked before launching it
6+
Intent forwardIntent = (Intent) getIntent().getParcelableExtra("forward_intent");
7+
ComponentName destinationComponent = forwardIntent.resolveActivity(getPackageManager());
8+
if (destinationComponent.getPackageName().equals("safe.package") &&
9+
destinationComponent.getClassName().equals("SafeClass")) {
10+
startActivity(forwardIntent);
11+
}
12+
13+
// GOOD: The component that sent the Intent is checked before launching the destination component
14+
Intent forwardIntent = (Intent) getIntent().getParcelableExtra("forward_intent");
15+
ComponentName originComponent = getCallingActivity();
16+
if (originComponent.getPackageName().equals("trusted.package") && originComponent.getClassName().equals("TrustedClass")) {
17+
startActivity(forwardIntent);
18+
}

‎java/ql/test/library-tests/dataflow/taintsources/IntentSources.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ public void test3() throws java.io.IOException {
2929

3030
}
3131

32-
3332
class OtherClass {
3433

3534
private static void sink(Object o) {}

0 commit comments

Comments
 (0)