Skip to content

Commit 9a80ab3

Browse files
smowtonsmowton
authored
Merge pull request #6567 from luchua-bc/java/sensitive_android_file_leak
Java: CWE-200 - Query to detect exposure of sensitive information from android file intent
2 parents 4b069d4 + 39640ef commit 9a80ab3

File tree

26 files changed

+2332
-39
lines changed

26 files changed

+2332
-39
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,19 @@ private class NumberTaintPreservingCallable extends TaintPreservingCallable {
103103

104104
override predicate returnsTaintFrom(int arg) { arg = argument }
105105
}
106+
107+
/**
108+
* A `Content` that should be implicitly regarded as tainted whenever an object with such `Content`
109+
* is itself tainted.
110+
*
111+
* For example, if we had a type `class Container { Contained field; }`, then by default a tainted
112+
* `Container` and a `Container` with a tainted `Contained` stored in its `field` are distinct.
113+
*
114+
* If `any(DataFlow::FieldContent fc | fc.getField().hasQualifiedName("Container", "field"))` was
115+
* included in this type however, then a tainted `Container` would imply that its `field` is also
116+
* tainted (but not vice versa).
117+
*
118+
* Note that `TaintTracking::Configuration` applies this behaviour by default to array, collection,
119+
* map-key and map-value content, so that e.g. a tainted `Map` is assumed to have tainted keys and values.
120+
*/
121+
abstract class TaintInheritingContent extends DataFlow::Content { }

‎java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,11 @@ private module Cached {
8383
not sink.getTypeBound() instanceof PrimitiveType and
8484
not sink.getTypeBound() instanceof BoxedType and
8585
not sink.getTypeBound() instanceof NumberType and
86-
containerContent(f)
86+
(
87+
containerContent(f)
88+
or
89+
f instanceof TaintInheritingContent
90+
)
8791
)
8892
or
8993
FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false)

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

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import java
2+
private import semmle.code.java.dataflow.DataFlow
23
import semmle.code.java.dataflow.FlowSteps
34
import semmle.code.java.dataflow.ExternalFlow
45

@@ -35,23 +36,12 @@ class ContextStartActivityMethod extends Method {
3536
}
3637
}
3738

38-
class IntentGetExtraMethod extends Method, TaintPreservingCallable {
39-
IntentGetExtraMethod() {
40-
(getName().regexpMatch("get\\w+Extra") or hasName("getExtras")) and
41-
getDeclaringType() instanceof TypeIntent
42-
}
43-
44-
override predicate returnsTaintFrom(int arg) { arg = -1 }
45-
}
46-
47-
/** A getter on `android.os.BaseBundle` or `android.os.Bundle`. */
48-
class BundleGetterMethod extends Method, TaintPreservingCallable {
49-
BundleGetterMethod() {
50-
getDeclaringType().hasQualifiedName("android.os", ["BaseBundle", "Bundle"]) and
51-
getName().matches("get%")
52-
}
53-
54-
override predicate returnsTaintFrom(int arg) { arg = -1 }
39+
/**
40+
* Specifies that if an `Intent` is tainted, then so are its synthetic fields.
41+
*/
42+
private class IntentFieldsInheritTaint extends DataFlow::SyntheticFieldContent,
43+
TaintInheritingContent {
44+
IntentFieldsInheritTaint() { this.getField().matches("android.content.Intent.%") }
5545
}
5646

5747
private class IntentBundleFlowSteps extends SummaryModelCsv {
@@ -142,31 +132,45 @@ private class IntentBundleFlowSteps extends SummaryModelCsv {
142132
"android.os;Bundle;true;putStringArrayList;;;Argument[1];MapValue of Argument[-1];value",
143133
"android.os;Bundle;true;readFromParcel;;;Argument[0];MapKey of Argument[-1];taint",
144134
"android.os;Bundle;true;readFromParcel;;;Argument[0];MapValue of Argument[-1];taint",
145-
// currently only the Extras part of the intent is fully modelled
146-
"android.content;Intent;true;addCategory;;;Argument[-1];ReturnValue;value",
147-
"android.content;Intent;true;addFlags;;;Argument[-1];ReturnValue;value",
135+
// currently only the Extras part of the intent and the data field are fully modelled
148136
"android.content;Intent;false;Intent;(Intent);;MapKey of SyntheticField[android.content.Intent.extras] of Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
149137
"android.content;Intent;false;Intent;(Intent);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[0];MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
150-
"android.content;Intent;true;getExtras;();;SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
138+
"android.content;Intent;false;Intent;(String,Uri);;Argument[1];SyntheticField[android.content.Intent.data] of Argument[-1];value",
139+
"android.content;Intent;false;Intent;(String,Uri,Context,Class);;Argument[1];SyntheticField[android.content.Intent.data] of Argument[-1];value",
140+
"android.content;Intent;true;addCategory;;;Argument[-1];ReturnValue;value",
141+
"android.content;Intent;true;addFlags;;;Argument[-1];ReturnValue;value",
142+
"android.content;Intent;false;createChooser;;;Argument[0..2];MapValue of SyntheticField[android.content.Intent.extras] of ReturnValue;value",
151143
"android.content;Intent;true;getBundleExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
152144
"android.content;Intent;true;getByteArrayExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
153145
"android.content;Intent;true;getCharArrayExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
154146
"android.content;Intent;true;getCharSequenceArrayExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
155147
"android.content;Intent;true;getCharSequenceArrayListExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
156148
"android.content;Intent;true;getCharSequenceExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
149+
"android.content;Intent;true;getData;;;SyntheticField[android.content.Intent.data] of Argument[-1];ReturnValue;value",
150+
"android.content;Intent;true;getDataString;;;SyntheticField[android.content.Intent.data] of Argument[-1];ReturnValue;taint",
151+
"android.content;Intent;true;getExtras;();;SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
152+
"android.content;Intent;false;getIntent;;;Argument[0];SyntheticField[android.content.Intent.data] of ReturnValue;taint",
153+
"android.content;Intent;false;getIntentOld;;;Argument[0];SyntheticField[android.content.Intent.data] of ReturnValue;taint",
157154
"android.content;Intent;true;getParcelableArrayExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
158155
"android.content;Intent;true;getParcelableArrayListExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
159156
"android.content;Intent;true;getParcelableExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
160157
"android.content;Intent;true;getSerializableExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
161158
"android.content;Intent;true;getStringArrayExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
162159
"android.content;Intent;true;getStringArrayListExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
163160
"android.content;Intent;true;getStringExtra;(String);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];ReturnValue;value",
161+
"android.content;Intent;false;parseUri;;;Argument[0];SyntheticField[android.content.Intent.data] of ReturnValue;taint",
164162
"android.content;Intent;true;putCharSequenceArrayListExtra;;;Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
165163
"android.content;Intent;true;putCharSequenceArrayListExtra;;;Argument[1];MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
166164
"android.content;Intent;true;putCharSequenceArrayListExtra;;;Argument[-1];ReturnValue;value",
167165
"android.content;Intent;true;putExtra;;;Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
168166
"android.content;Intent;true;putExtra;;;Argument[1];MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
169167
"android.content;Intent;true;putExtra;;;Argument[-1];ReturnValue;value",
168+
"android.content;Intent;true;putExtras;(Bundle);;MapKey of Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
169+
"android.content;Intent;true;putExtras;(Bundle);;MapValue of Argument[0];MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
170+
"android.content;Intent;true;putExtras;(Bundle);;Argument[-1];ReturnValue;value",
171+
"android.content;Intent;true;putExtras;(Intent);;MapKey of SyntheticField[android.content.Intent.extras] of Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
172+
"android.content;Intent;true;putExtras;(Intent);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[0];MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
173+
"android.content;Intent;true;putExtras;(Intent);;Argument[-1];ReturnValue;value",
170174
"android.content;Intent;true;putIntegerArrayListExtra;;;Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
171175
"android.content;Intent;true;putIntegerArrayListExtra;;;Argument[-1];ReturnValue;value",
172176
"android.content;Intent;true;putParcelableArrayListExtra;;;Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
@@ -175,12 +179,6 @@ private class IntentBundleFlowSteps extends SummaryModelCsv {
175179
"android.content;Intent;true;putStringArrayListExtra;;;Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
176180
"android.content;Intent;true;putStringArrayListExtra;;;Argument[1];MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
177181
"android.content;Intent;true;putStringArrayListExtra;;;Argument[-1];ReturnValue;value",
178-
"android.content;Intent;true;putExtras;(Bundle);;MapKey of Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
179-
"android.content;Intent;true;putExtras;(Bundle);;MapValue of Argument[0];MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
180-
"android.content;Intent;true;putExtras;(Bundle);;Argument[-1];ReturnValue;value",
181-
"android.content;Intent;true;putExtras;(Intent);;MapKey of SyntheticField[android.content.Intent.extras] of Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
182-
"android.content;Intent;true;putExtras;(Intent);;MapValue of SyntheticField[android.content.Intent.extras] of Argument[0];MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
183-
"android.content;Intent;true;putExtras;(Intent);;Argument[-1];ReturnValue;value",
184182
"android.content;Intent;true;replaceExtras;(Bundle);;MapKey of Argument[0];MapKey of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
185183
"android.content;Intent;true;replaceExtras;(Bundle);;MapValue of Argument[0];MapValue of SyntheticField[android.content.Intent.extras] of Argument[-1];value",
186184
"android.content;Intent;true;replaceExtras;(Bundle);;Argument[-1];ReturnValue;value",
@@ -192,9 +190,13 @@ private class IntentBundleFlowSteps extends SummaryModelCsv {
192190
"android.content;Intent;true;setClassName;;;Argument[-1];ReturnValue;value",
193191
"android.content;Intent;true;setComponent;;;Argument[-1];ReturnValue;value",
194192
"android.content;Intent;true;setData;;;Argument[-1];ReturnValue;value",
193+
"android.content;Intent;true;setData;;;Argument[0];SyntheticField[android.content.Intent.data] of Argument[-1];value",
195194
"android.content;Intent;true;setDataAndNormalize;;;Argument[-1];ReturnValue;value",
195+
"android.content;Intent;true;setDataAndNormalize;;;Argument[0];SyntheticField[android.content.Intent.data] of Argument[-1];value",
196196
"android.content;Intent;true;setDataAndType;;;Argument[-1];ReturnValue;value",
197+
"android.content;Intent;true;setDataAndType;;;Argument[0];SyntheticField[android.content.Intent.data] of Argument[-1];value",
197198
"android.content;Intent;true;setDataAndTypeAndNormalize;;;Argument[-1];ReturnValue;value",
199+
"android.content;Intent;true;setDataAndTypeAndNormalize;;;Argument[0];SyntheticField[android.content.Intent.data] of Argument[-1];value",
198200
"android.content;Intent;true;setFlags;;;Argument[-1];ReturnValue;value",
199201
"android.content;Intent;true;setIdentifier;;;Argument[-1];ReturnValue;value",
200202
"android.content;Intent;true;setPackage;;;Argument[-1];ReturnValue;value",

‎java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSink.qll

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/** Provides Android sink models related to file creation. */
2+
3+
import java
4+
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.java.dataflow.ExternalFlow
6+
import semmle.code.java.frameworks.android.Android
7+
import semmle.code.java.frameworks.android.Intent
8+
9+
/** A sink representing methods creating a file in Android. */
10+
class AndroidFileSink extends DataFlow::Node {
11+
AndroidFileSink() { sinkNode(this, "create-file") }
12+
}
13+
14+
/**
15+
* The Android class `android.os.AsyncTask` for running tasks off the UI thread to achieve
16+
* better user experience.
17+
*/
18+
class AsyncTask extends RefType {
19+
AsyncTask() { this.hasQualifiedName("android.os", "AsyncTask") }
20+
}
21+
22+
/** The `execute` or `executeOnExecutor` method of Android's `AsyncTask` class. */
23+
class ExecuteAsyncTaskMethod extends Method {
24+
int paramIndex;
25+
26+
ExecuteAsyncTaskMethod() {
27+
this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
28+
(
29+
this.getName() = "execute" and paramIndex = 0
30+
or
31+
this.getName() = "executeOnExecutor" and paramIndex = 1
32+
)
33+
}
34+
35+
int getParamIndex() { result = paramIndex }
36+
}
37+
38+
/** The `doInBackground` method of Android's `AsyncTask` class. */
39+
class AsyncTaskRunInBackgroundMethod extends Method {
40+
AsyncTaskRunInBackgroundMethod() {
41+
this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
42+
this.getName() = "doInBackground"
43+
}
44+
}
45+
46+
/** The service start method of Android's `Context` class. */
47+
class ContextStartServiceMethod extends Method {
48+
ContextStartServiceMethod() {
49+
this.getName() = ["startService", "startForegroundService"] and
50+
this.getDeclaringType().getASupertype*() instanceof TypeContext
51+
}
52+
}
53+
54+
/** The `onStartCommand` method of Android's `Service` class. */
55+
class ServiceOnStartCommandMethod extends Method {
56+
ServiceOnStartCommandMethod() {
57+
this.hasName("onStartCommand") and
58+
this.getDeclaringType() instanceof AndroidService
59+
}
60+
}

‎java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/** Provides summary models relating to file content inputs of Android. */
2+
3+
import java
4+
import semmle.code.java.dataflow.FlowSources
5+
import semmle.code.java.dataflow.TaintTracking2
6+
import semmle.code.java.frameworks.android.Android
7+
8+
/** The `startActivityForResult` method of Android's `Activity` class. */
9+
class StartActivityForResultMethod extends Method {
10+
StartActivityForResultMethod() {
11+
this.getDeclaringType().getASupertype*() instanceof AndroidActivity and
12+
this.getName() = "startActivityForResult"
13+
}
14+
}
15+
16+
/** An instance of `android.content.Intent` constructed passing `GET_CONTENT` to the constructor. */
17+
class GetContentIntent extends ClassInstanceExpr {
18+
GetContentIntent() {
19+
this.getConstructedType() instanceof TypeIntent and
20+
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() =
21+
"android.intent.action.GET_CONTENT"
22+
or
23+
exists(Field f |
24+
this.getArgument(0) = f.getAnAccess() and
25+
f.hasName("ACTION_GET_CONTENT") and
26+
f.getDeclaringType() instanceof TypeIntent
27+
)
28+
}
29+
}
30+
31+
/** Taint configuration that identifies `GET_CONTENT` `Intent` instances passed to `startActivityForResult`. */
32+
class GetContentIntentConfig extends TaintTracking2::Configuration {
33+
GetContentIntentConfig() { this = "GetContentIntentConfig" }
34+
35+
override predicate isSource(DataFlow2::Node src) {
36+
exists(GetContentIntent gi | src.asExpr() = gi)
37+
}
38+
39+
override predicate isSink(DataFlow2::Node sink) {
40+
exists(MethodAccess ma |
41+
ma.getMethod() instanceof StartActivityForResultMethod and sink.asExpr() = ma.getArgument(0)
42+
)
43+
}
44+
45+
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content content) {
46+
super.allowImplicitRead(node, content)
47+
or
48+
// Allow the wrapped intent created by Intent.getChooser to be consumed
49+
// by at the sink:
50+
isSink(node) and
51+
(
52+
content.(DataFlow::SyntheticFieldContent).getField() = "android.content.Intent.extras"
53+
or
54+
content instanceof DataFlow::MapValueContent
55+
)
56+
}
57+
}
58+
59+
/** A `GET_CONTENT` `Intent` instances that is passed to `startActivityForResult`. */
60+
class AndroidFileIntentInput extends DataFlow::Node {
61+
MethodAccess ma;
62+
63+
AndroidFileIntentInput() {
64+
this.asExpr() = ma.getArgument(0) and
65+
ma.getMethod() instanceof StartActivityForResultMethod and
66+
exists(GetContentIntentConfig cc, GetContentIntent gi |
67+
cc.hasFlow(DataFlow::exprNode(gi), DataFlow::exprNode(ma.getArgument(0)))
68+
)
69+
}
70+
71+
/** The request code passed to `startActivityForResult`, which is to be matched in `onActivityResult()`. */
72+
int getRequestCode() { result = ma.getArgument(1).(CompileTimeConstantExpr).getIntValue() }
73+
}
74+
75+
/** The `onActivityForResult` method of Android `Activity` */
76+
class OnActivityForResultMethod extends Method {
77+
OnActivityForResultMethod() {
78+
this.getDeclaringType().getASupertype*() instanceof AndroidActivity and
79+
this.getName() = "onActivityResult"
80+
}
81+
}

‎java/ql/src/experimental/Security/CWE/CWE-200/LoadFileFromAppActivity.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
public class LoadFileFromAppActivity extends Activity {
2+
public static final int REQUEST_CODE__SELECT_CONTENT_FROM_APPS = 99;
3+
4+
@Override
5+
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
6+
if (requestCode == LoadFileFromAppActivity.REQUEST_CODE__SELECT_CONTENT_FROM_APPS &&
7+
resultCode == RESULT_OK) {
8+
9+
{
10+
// BAD: Load file without validation
11+
loadOfContentFromApps(data, resultCode);
12+
}
13+
14+
{
15+
// GOOD: load file with validation
16+
if (!data.getData().getPath().startsWith("/data/data")) {
17+
loadOfContentFromApps(data, resultCode);
18+
}
19+
}
20+
}
21+
}
22+
23+
private void loadOfContentFromApps(Intent contentIntent, int resultCode) {
24+
Uri streamsToUpload = contentIntent.getData();
25+
try {
26+
RandomAccessFile file = new RandomAccessFile(streamsToUpload.getPath(), "r");
27+
} catch (Exception ex) {
28+
ex.printStackTrace();
29+
}
30+
}
31+
}

0 commit comments

Comments
 (0)