Skip to content

Commit d0077b8

Browse files
atorralbaatorralba
committed
Added query ImplicitPendingIntents
1 parent 68385df commit d0077b8

File tree

8 files changed

+405
-0
lines changed

8 files changed

+405
-0
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ private module Frameworks {
116116
private import semmle.code.java.security.ResponseSplitting
117117
private import semmle.code.java.security.InformationLeak
118118
private import semmle.code.java.security.GroovyInjection
119+
private import semmle.code.java.security.ImplicitPendingIntents
119120
private import semmle.code.java.security.JexlInjectionSinkModels
120121
private import semmle.code.java.security.JndiInjection
121122
private import semmle.code.java.security.LdapInjection

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ class AndroidBundle extends Class {
8787
AndroidBundle() { this.getASupertype*().hasQualifiedName("android.os", "BaseBundle") }
8888
}
8989

90+
/** The class `android.app.PendingIntent`. */
91+
class PendingIntent extends Class {
92+
PendingIntent() { this.hasQualifiedName("android.app", "PendingIntent") }
93+
}
94+
9095
/** An `Intent` that explicitly sets a destination component. */
9196
class ExplicitIntent extends Expr {
9297
ExplicitIntent() {

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

Lines changed: 171 additions & 0 deletions
Large diffs are not rendered by default.

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

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/** Provides taint tracking configurations to be used in queries related to implicit `PendingIntent`s. */
2+
3+
import java
4+
import semmle.code.java.dataflow.ExternalFlow
5+
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.frameworks.android.Intent
7+
import semmle.code.java.security.ImplicitPendingIntents
8+
9+
/**
10+
* A taint tracking configuration for implicit `PendingIntent`s
11+
* being wrapped in another implicit `Intent` that gets started.
12+
*/
13+
class ImplicitPendingIntentStartConf extends TaintTracking::Configuration {
14+
ImplicitPendingIntentStartConf() { this = "ImplicitPendingIntentStartConf" }
15+
16+
override predicate isSource(DataFlow::Node source) {
17+
source.asExpr() instanceof ImplicitPendingIntentCreation
18+
}
19+
20+
override predicate isSink(DataFlow::Node sink) {
21+
sink instanceof IntentStartSink and
22+
// startService can't actually start implicit intents since API 21
23+
not exists(MethodAccess ma, Method m |
24+
ma.getMethod() = m and
25+
m.getDeclaringType().getASupertype*() instanceof TypeContext and
26+
m.hasName("startService") and
27+
sink.asExpr() = ma.getArgument(0)
28+
)
29+
}
30+
31+
override predicate isSanitizer(DataFlow::Node sanitizer) {
32+
sanitizer instanceof ExplicitIntentSanitizer
33+
}
34+
35+
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
36+
super.allowImplicitRead(node, c)
37+
or
38+
this.isSink(node)
39+
}
40+
}
41+
42+
private class ImplicitPendingIntentCreation extends Expr {
43+
ImplicitPendingIntentCreation() {
44+
exists(Argument arg |
45+
this.getType() instanceof PendingIntent and
46+
exists(ImplicitPendingIntentConf conf | conf.hasFlowTo(DataFlow::exprNode(arg))) and
47+
arg.getCall() = this
48+
)
49+
}
50+
}
51+
52+
private class IntentStartSink extends DataFlow::Node {
53+
IntentStartSink() { sinkNode(this, "intent-start") }
54+
}
55+
56+
private class ImplicitPendingIntentConf extends DataFlow2::Configuration {
57+
ImplicitPendingIntentConf() { this = "PendingIntentConf" }
58+
59+
override predicate isSource(DataFlow::Node source) {
60+
exists(ClassInstanceExpr cc |
61+
cc.getConstructedType() instanceof TypeIntent and source.asExpr() = cc
62+
)
63+
}
64+
65+
override predicate isSink(DataFlow::Node sink) { sink instanceof MutablePendingIntentSink }
66+
67+
override predicate isBarrier(DataFlow::Node barrier) {
68+
barrier instanceof ExplicitIntentSanitizer
69+
}
70+
}
71+
72+
private class PendingIntentSink extends DataFlow::Node {
73+
PendingIntentSink() { sinkNode(this, "pending-intent") }
74+
}
75+
76+
private class MutablePendingIntentSink extends PendingIntentSink {
77+
MutablePendingIntentSink() {
78+
exists(Argument flagArgument |
79+
flagArgument = this.asExpr().(Argument).getCall().getArgument(3)
80+
|
81+
// API < 31, PendingIntents are mutable by default
82+
not TaintTracking::localExprTaint(getPendingIntentFlagAccess("FLAG_IMMUTABLE"), flagArgument)
83+
or
84+
// API >= 31, PendingIntents need to explicitly set mutability
85+
TaintTracking::localExprTaint(getPendingIntentFlagAccess("FLAG_MUTABLE"), flagArgument)
86+
)
87+
}
88+
}
89+
90+
private Expr getPendingIntentFlagAccess(string flagName) {
91+
exists(Field f |
92+
f.getDeclaringType() instanceof PendingIntent and
93+
f.isPublic() and
94+
f.isFinal() and
95+
f.hasName(flagName) and
96+
f.getAnAccess() = result
97+
)
98+
}

‎java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import android.app.Activity;
2+
import android.app.PendingIntent;
3+
import android.content.Intent;
4+
import android.os.Bundle;
5+
6+
public class ImplicitPendingIntents extends Activity {
7+
8+
public void onCreate(Bundle savedInstance) {
9+
{
10+
// BAD: an implicit Intent is used to create a PendingIntent.
11+
// The PendingIntent is then added to another implicit Intent
12+
// and started.
13+
Intent baseIntent = new Intent();
14+
PendingIntent pi =
15+
PendingIntent.getActivity(this, 0, baseIntent, PendingIntent.FLAG_ONE_SHOT);
16+
Intent fwdIntent = new Intent("SOME_ACTION");
17+
fwdIntent.putExtra("fwdIntent", pi);
18+
sendBroadcast(fwdIntent);
19+
}
20+
21+
{
22+
// GOOD: both the PendingIntent and the wrapping Intent are explicit.
23+
Intent safeIntent = new Intent(this, AnotherActivity.class);
24+
PendingIntent pi =
25+
PendingIntent.getActivity(this, 0, safeIntent, PendingIntent.FLAG_ONE_SHOT);
26+
Intent fwdIntent = new Intent();
27+
fwdIntent.setClassName("destination.package", "DestinationClass");
28+
fwdIntent.putExtra("fwdIntent", pi);
29+
startActivity(fwdIntent);
30+
}
31+
32+
{
33+
// GOOD: The PendingIntent is created with FLAG_IMMUTABLE.
34+
Intent baseIntent = new Intent("SOME_ACTION");
35+
PendingIntent pi =
36+
PendingIntent.getActivity(this, 0, baseIntent, PendingIntent.FLAG_IMMUTABLE);
37+
Intent fwdIntent = new Intent();
38+
fwdIntent.setClassName("destination.package", "DestinationClass");
39+
fwdIntent.putExtra("fwdIntent", pi);
40+
startActivity(fwdIntent);
41+
}
42+
}
43+
}

‎java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.qhelp

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
5+
<qhelp>
6+
7+
<overview>
8+
<p>A <code>PendingIntent</code> describes an action in the form of an Intent that is intended to be given and executed
9+
at a later time by another application. The Intent wrapped by a <code>PendingIntent</code> is executed on behalf of
10+
the application that created it, and with its same privileges.</p>
11+
<p>If a <code>PendingIntent</code> is configured to be mutable, the fields of its internal Intent can be changed by the
12+
receiving application if they were not previously set. This means that a mutable <code>PendingIntent</code> that has
13+
not defined a destination component (that is, an implicit <code>PendingIntent</code>) can be directed to any component
14+
by the receiving application, and execute an arbitrary action with the privileges of the application that created it.</p>
15+
<p>If an implicit <code>PendingIntent</code> is wrapped and sent as an extra of an Intent that can be intercepted (that
16+
is, again, an implicit Intent), any malicious application could obtain the <code>PendingIntent</code>, modify the
17+
underlying Intent with an arbitrary destination component, and execute the desired action with elevated privileges.
18+
This could give the malicious application access to private components of the victim application, or the ability to
19+
perform actions without having the necessary permissions.</p>
20+
</overview>
21+
22+
<recommendation>
23+
<p>Avoid creating implicit <code>PendingIntent</code>s. This means that the underlying Intent should always have an
24+
explicit destination component.</p>
25+
<p>Also, when adding the <code>PendingIntent</code> as an extra of another Intent, make sure that said Intent also has
26+
an explicit destination component, so that it is not delivered to untrusted applications.</p>
27+
<p>It is also recommended to create the <code>PendingIntent</code> using the flag <code>FLAG_IMMUTABLE</code> whenever
28+
possible, to prevent the destination component from modifying empty fields of the underlying Intent.</p>
29+
</recommendation>
30+
31+
<example>
32+
<p>In the following examples, a <code>PendingIntent</code> is created and wrapped as an extra of another Intent.
33+
</p>
34+
<p>In the first example, both the <code>PendingIntent</code> and the Intent it is wrapped in are implicit,
35+
reproducing the vulnerability.</p>
36+
<p>In the second example, the issue is avoided by adding explicit destination components to the
37+
<code>PendingIntent</code> and the wrapping Intent.</p>
38+
<p>The third example uses the <code>FLAG_IMMUTABLE</code> flag to prevent the underlying Intent from being modified
39+
by the destination component.</p>
40+
<sample src="ImplicitPendingIntents.java" />
41+
</example>
42+
43+
<references>
44+
<li>
45+
Google Help:
46+
<a href="https://support.google.com/faqs/answer/10437428?hl=en">
47+
Remediation for Implicit PendingIntent Vulnerability
48+
</a>
49+
</li>
50+
<li>
51+
University of Potsdam:
52+
<a href="https://www.cs.uni-potsdam.de/se/papers/esorics18.pdf">
53+
PIAnalyzer: A precise approach for PendingIntent vulnerability analysis
54+
</a>
55+
</li>
56+
</references>
57+
58+
</qhelp>

‎java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.ql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @name Use of implicit Pending Intents
3+
* @description Implicit and mutable PendingIntents being wrapped and sent in another implicit
4+
* Intent may provide access to internal components of the application or cause other
5+
* unintended effects.
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @security-severity 8.2
9+
* @precision high
10+
* @id java/android/pending-intents
11+
* @tags security
12+
* external/cwe/cwe-927
13+
*/
14+
15+
import java
16+
import semmle.code.java.dataflow.DataFlow
17+
import semmle.code.java.security.ImplicitPendingIntentsQuery
18+
import DataFlow::PathGraph
19+
20+
from DataFlow::PathNode source, DataFlow::PathNode sink
21+
where any(ImplicitPendingIntentStartConf conf).hasFlowPath(source, sink)
22+
select sink.getNode(), source, sink, "something"

‎java/ql/src/change-notes/2021-09-30-android-implicit-pendingintents.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
category: newQuery
3+
---
4+
* A new query "Use of implicit Pending Intents" (`java/android/pending-intents`) has been added.
5+
This query finds implicit and mutable PendingIntents being wrapped and sent in another implicit Intent,
6+
which can provide access to internal components of the application or cause other unintended
7+
effects.

0 commit comments

Comments
 (0)