Skip to content

Commit 89361a3

Browse files
aschackmullaschackmull
authored
Merge pull request github#3812 from luchua-bc/java-android-remote-source
Java: Add remote source of Android intent extra
2 parents 82f37e9 + 864411b commit 89361a3

File tree

25 files changed

+1065
-41
lines changed

25 files changed

+1065
-41
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.ql‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -58,27 +58,6 @@ class FetchResourceMethodAccess extends MethodAccess {
5858
}
5959
}
6060

61-
/**
62-
* Method access to external inputs of `android.content.Intent` object
63-
*/
64-
class IntentGetExtraMethodAccess extends MethodAccess {
65-
IntentGetExtraMethodAccess() {
66-
this.getMethod().getName().regexpMatch("get\\w+Extra") and
67-
this.getMethod().getDeclaringType() instanceof TypeIntent
68-
or
69-
this.getMethod().getName().regexpMatch("get\\w+") and
70-
this.getQualifier().(MethodAccess).getMethod().hasName("getExtras") and
71-
this.getQualifier().(MethodAccess).getMethod().getDeclaringType() instanceof TypeIntent
72-
}
73-
}
74-
75-
/**
76-
* Source of fetching URLs from intent extras
77-
*/
78-
class UntrustedResourceSource extends DataFlow::ExprNode {
79-
UntrustedResourceSource() { this.asExpr() instanceof IntentGetExtraMethodAccess }
80-
}
81-
8261
/**
8362
* Holds if `ma` loads URL `sink`
8463
*/
@@ -127,7 +106,7 @@ class UrlResourceSink extends DataFlow::ExprNode {
127106
class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
128107
FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }
129108

130-
override predicate isSource(DataFlow::Node source) { source instanceof UntrustedResourceSource }
109+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
131110

132111
override predicate isSink(DataFlow::Node sink) {
133112
sink instanceof UrlResourceSink and

‎java/ql/src/semmle/code/java/dataflow/FlowSources.qll‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import semmle.code.java.frameworks.android.XmlParsing
1616
import semmle.code.java.frameworks.android.WebView
1717
import semmle.code.java.frameworks.JaxWS
1818
import semmle.code.java.frameworks.javase.WebSocket
19+
import semmle.code.java.frameworks.android.Android
1920
import semmle.code.java.frameworks.android.Intent
2021
import semmle.code.java.frameworks.spring.SpringWeb
2122
import semmle.code.java.frameworks.spring.SpringController
@@ -323,15 +324,26 @@ class ReverseDNSMethod extends Method {
323324

324325
/** Android `Intent` that may have come from a hostile application. */
325326
class AndroidIntentInput extends DataFlow::Node {
327+
Type receiverType;
328+
326329
AndroidIntentInput() {
327330
exists(MethodAccess ma, AndroidGetIntentMethod m |
328331
ma.getMethod().overrides*(m) and
329-
this.asExpr() = ma
332+
this.asExpr() = ma and
333+
receiverType = ma.getReceiverType()
330334
)
331335
or
332336
exists(Method m, AndroidReceiveIntentMethod rI |
333337
m.overrides*(rI) and
334-
this.asParameter() = m.getParameter(1)
338+
this.asParameter() = m.getParameter(1) and
339+
receiverType = m.getDeclaringType()
335340
)
336341
}
337342
}
343+
344+
/** Exported Android `Intent` that may have come from a hostile application. */
345+
class ExportedAndroidIntentInput extends RemoteFlowSource, AndroidIntentInput {
346+
ExportedAndroidIntentInput() { receiverType.(ExportableAndroidComponent).isExported() }
347+
348+
override string getSourceType() { result = "Exported Android intent source" }
349+
}

‎java/ql/src/semmle/code/java/frameworks/android/Android.qll‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,42 @@ class AndroidComponent extends Class {
3030
predicate hasIntentFilter() { exists(getAndroidComponentXmlElement().getAnIntentFilterElement()) }
3131
}
3232

33+
/**
34+
* An Android component that can be explicitly or implicitly exported.
35+
*/
36+
class ExportableAndroidComponent extends AndroidComponent {
37+
/**
38+
* Holds if this Android component is configured as `exported` or has intent
39+
* filters configured without `exported` explicitly disabled in an
40+
* `AndroidManifest.xml` file.
41+
*/
42+
override predicate isExported() {
43+
getAndroidComponentXmlElement().isExported()
44+
or
45+
hasIntentFilter() and
46+
not getAndroidComponentXmlElement().isNotExported()
47+
}
48+
}
49+
3350
/** An Android activity. */
34-
class AndroidActivity extends AndroidComponent {
51+
class AndroidActivity extends ExportableAndroidComponent {
3552
AndroidActivity() { this.getASupertype*().hasQualifiedName("android.app", "Activity") }
3653
}
3754

3855
/** An Android service. */
39-
class AndroidService extends AndroidComponent {
56+
class AndroidService extends ExportableAndroidComponent {
4057
AndroidService() { this.getASupertype*().hasQualifiedName("android.app", "Service") }
4158
}
4259

4360
/** An Android broadcast receiver. */
44-
class AndroidBroadcastReceiver extends AndroidComponent {
61+
class AndroidBroadcastReceiver extends ExportableAndroidComponent {
4562
AndroidBroadcastReceiver() {
4663
this.getASupertype*().hasQualifiedName("android.content", "BroadcastReceiver")
4764
}
4865
}
4966

5067
/** An Android content provider. */
51-
class AndroidContentProvider extends AndroidComponent {
68+
class AndroidContentProvider extends ExportableAndroidComponent {
5269
AndroidContentProvider() {
5370
this.getASupertype*().hasQualifiedName("android.content", "ContentProvider")
5471
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,13 @@ class IntentGetExtraMethod extends Method, TaintPreservingCallable {
4242

4343
override predicate returnsTaintFrom(int arg) { arg = -1 }
4444
}
45+
46+
/** A getter on `android.os.BaseBundle` or `android.os.Bundle`. */
47+
class BundleGetterMethod extends Method, TaintPreservingCallable {
48+
BundleGetterMethod() {
49+
getDeclaringType().hasQualifiedName("android.os", ["BaseBundle", "Bundle"]) and
50+
getName().matches("get%")
51+
}
52+
53+
override predicate returnsTaintFrom(int arg) { arg = -1 }
54+
}

‎java/ql/src/semmle/code/xml/AndroidManifest.qll‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,11 @@ class AndroidComponentXmlElement extends XMLElement {
137137
* Holds if the `android:exported` attribute of this component element is `true`.
138138
*/
139139
predicate isExported() { getExportedAttributeValue() = "true" }
140+
141+
/**
142+
* Holds if the `android:exported` attribute of this component element is explicitly set to `false`.
143+
*/
144+
predicate isNotExported() { getExportedAttributeValue() = "false" }
140145
}
141146

142147
/**

‎java/ql/test/experimental/query-tests/security/CWE-749/AndroidManifest.xml‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
2+
package="com.example.app"
3+
android:installLocation="auto"
4+
android:versionCode="1"
5+
android:versionName="0.1" >
6+
7+
<uses-permission android:name="android.permission.INTERNET" />
8+
9+
<application
10+
android:icon="@drawable/ic_launcher"
11+
android:label="@string/app_name"
12+
android:theme="@style/AppTheme" >
13+
<activity
14+
android:name=".UnsafeAndroidAccess"
15+
android:icon="@drawable/ic_launcher"
16+
android:label="@string/app_name">
17+
<intent-filter>
18+
<action android:name="android.intent.action.MAIN" />
19+
<category android:name="android.intent.category.LAUNCHER" />
20+
</intent-filter>
21+
</activity>
22+
23+
<activity android:name=".UnsafeActivity1" android:exported="true">
24+
<intent-filter>
25+
<action android:name="android.intent.action.VIEW"/>
26+
</intent-filter>
27+
</activity>
28+
29+
<activity android:name=".UnsafeActivity2">
30+
<intent-filter>
31+
<action android:name="android.intent.action.VIEW"/>
32+
</intent-filter>
33+
</activity>
34+
35+
<activity android:name=".SafeActivity1" android:exported="false">
36+
<intent-filter>
37+
<action android:name="android.intent.action.VIEW"/>
38+
</intent-filter>
39+
</activity>
40+
41+
<activity android:name=".SafeActivity2" android:exported="false" />
42+
43+
<activity android:name=".SafeActivity3" />
44+
45+
<activity android:name=".UnsafeActivity3" android:exported="true" />
46+
<activity android:name=".UnsafeActivity4" android:exported="true" />
47+
48+
<receiver android:name=".UnsafeAndroidBroadcastReceiver" android:exported="true" />
49+
</application>
50+
51+
</manifest>

‎java/ql/test/experimental/query-tests/security/CWE-749/IntentUtils.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package com.example.app;
2+
3+
import android.app.Activity;
4+
5+
import android.os.Bundle;
6+
7+
import android.webkit.WebSettings;
8+
import android.webkit.WebView;
9+
import android.webkit.WebViewClient;
10+
11+
/** A utility program for getting intent extra information from Android activity */
12+
public class IntentUtils {
13+
/** Get intent extra */
14+
public static String getIntentUrl(Activity a) {
15+
String thisUrl = a.getIntent().getStringExtra("url");
16+
return thisUrl;
17+
}
18+
19+
/** Get bundle extra */
20+
public static String getBundleUrl(Activity a) {
21+
String thisUrl = a.getIntent().getExtras().getString("url");
22+
return thisUrl;
23+
}
24+
}

‎java/ql/test/experimental/query-tests/security/CWE-749/SafeActivity1.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package com.example.app;
2+
3+
import android.app.Activity;
4+
5+
import android.os.Bundle;
6+
7+
import android.webkit.WebSettings;
8+
import android.webkit.WebView;
9+
import android.webkit.WebViewClient;
10+
11+
public class SafeActivity1 extends Activity {
12+
//Test onCreate with both JavaScript and cross-origin resource access enabled while taking remote user inputs from bundle extras
13+
public void onCreate(Bundle savedInstanceState) {
14+
super.onCreate(savedInstanceState);
15+
setContentView(-1);
16+
17+
WebView wv = (WebView) findViewById(-1);
18+
WebSettings webSettings = wv.getSettings();
19+
20+
webSettings.setJavaScriptEnabled(true);
21+
webSettings.setAllowFileAccessFromFileURLs(true);
22+
23+
wv.setWebViewClient(new WebViewClient() {
24+
@Override
25+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
26+
view.loadUrl(url);
27+
return true;
28+
}
29+
});
30+
31+
String thisUrl = getIntent().getExtras().getString("url");
32+
wv.loadUrl(thisUrl);
33+
}
34+
}

‎java/ql/test/experimental/query-tests/security/CWE-749/SafeActivity2.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package com.example.app;
2+
3+
import android.app.Activity;
4+
5+
import android.os.Bundle;
6+
7+
import android.webkit.WebSettings;
8+
import android.webkit.WebView;
9+
import android.webkit.WebViewClient;
10+
11+
public class SafeActivity2 extends Activity {
12+
//Test onCreate with both JavaScript and cross-origin resource access enabled while taking remote user inputs from bundle extras
13+
public void onCreate(Bundle savedInstanceState) {
14+
super.onCreate(savedInstanceState);
15+
setContentView(-1);
16+
17+
WebView wv = (WebView) findViewById(-1);
18+
WebSettings webSettings = wv.getSettings();
19+
20+
webSettings.setJavaScriptEnabled(true);
21+
webSettings.setAllowFileAccessFromFileURLs(true);
22+
23+
wv.setWebViewClient(new WebViewClient() {
24+
@Override
25+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
26+
view.loadUrl(url);
27+
return true;
28+
}
29+
});
30+
31+
String thisUrl = getIntent().getExtras().getString("url");
32+
wv.loadUrl(thisUrl);
33+
}
34+
}

‎java/ql/test/experimental/query-tests/security/CWE-749/SafeActivity3.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package com.example.app;
2+
3+
import android.app.Activity;
4+
5+
import android.os.Bundle;
6+
7+
import android.webkit.WebSettings;
8+
import android.webkit.WebView;
9+
import android.webkit.WebViewClient;
10+
11+
public class SafeActivity3 extends Activity {
12+
//Test onCreate with both JavaScript and cross-origin resource access enabled while taking remote user inputs from bundle extras
13+
public void onCreate(Bundle savedInstanceState) {
14+
super.onCreate(savedInstanceState);
15+
setContentView(-1);
16+
17+
WebView wv = (WebView) findViewById(-1);
18+
WebSettings webSettings = wv.getSettings();
19+
20+
webSettings.setJavaScriptEnabled(true);
21+
webSettings.setAllowFileAccessFromFileURLs(true);
22+
23+
wv.setWebViewClient(new WebViewClient() {
24+
@Override
25+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
26+
view.loadUrl(url);
27+
return true;
28+
}
29+
});
30+
31+
String thisUrl = getIntent().getExtras().getString("url");
32+
wv.loadUrl(thisUrl);
33+
}
34+
}

0 commit comments

Comments
 (0)