Skip to content

Commit 3cc3fe9

Browse files
luchua-bcluchua-bc
committed
Switch to TaintPreservingCallable and add test cases
1 parent 3f298f3 commit 3cc3fe9

File tree

15 files changed

+410
-60
lines changed

15 files changed

+410
-60
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: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -305,15 +305,29 @@ class ReverseDNSMethod extends Method {
305305
}
306306
}
307307

308-
/** Exported Android `Intent` that may have come from a hostile application. */
309-
class AndroidIntentInput extends RemoteFlowSource {
308+
/** Android `Intent` that may have come from a hostile application. */
309+
class AndroidIntentInput extends DataFlow::Node {
310310
AndroidIntentInput() {
311-
exists(AndroidComponent exportedType |
312-
exportedType.isExported() |
311+
exists(MethodAccess ma, AndroidGetIntentMethod m |
312+
ma.getMethod().overrides*(m) and
313+
this.asExpr() = ma
314+
)
315+
or
316+
exists(Method m, AndroidReceiveIntentMethod rI |
317+
m.overrides*(rI) and
318+
this.asParameter() = m.getParameter(1)
319+
)
320+
}
321+
}
322+
323+
/** Exported Android `Intent` that may have come from a hostile application. */
324+
class ExportedAndroidIntentInput extends RemoteFlowSource {
325+
ExportedAndroidIntentInput() {
326+
exists(ExportableAndroidComponent exportedType | exportedType.isExported() |
313327
exists(MethodAccess ma, AndroidGetIntentMethod m |
314328
ma.getMethod().overrides*(m) and
315329
this.asExpr() = ma and
316-
exportedType = ma.getReceiverType()
330+
exportedType = ma.getEnclosingCallable().getDeclaringType()
317331
)
318332
or
319333
exists(Method m, AndroidReceiveIntentMethod rI |

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

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

33-
/** An Android activity. */
34-
class AndroidActivity extends AndroidComponent {
35-
AndroidActivity() { this.getASupertype*().hasQualifiedName("android.app", "Activity") }
36-
33+
/**
34+
* An Android component that is explicitly or implicitly exported.
35+
*/
36+
class ExportableAndroidComponent extends AndroidComponent {
3737
/** Holds if this Android component is configured as `exported` or has intent filters configured without `exported` explicitly disabled in an `AndroidManifest.xml` file. */
3838
override predicate isExported() {
3939
getAndroidComponentXmlElement().isExported()
@@ -42,34 +42,25 @@ class AndroidActivity extends AndroidComponent {
4242
}
4343
}
4444

45+
/** An Android activity. */
46+
class AndroidActivity extends ExportableAndroidComponent {
47+
AndroidActivity() { this.getASupertype*().hasQualifiedName("android.app", "Activity") }
48+
}
49+
4550
/** An Android service. */
46-
class AndroidService extends AndroidComponent {
51+
class AndroidService extends ExportableAndroidComponent {
4752
AndroidService() { this.getASupertype*().hasQualifiedName("android.app", "Service") }
48-
49-
/** Holds if this Android component is configured as `exported` or has intent filters configured without `exported` explicitly disabled in an `AndroidManifest.xml` file. */
50-
override predicate isExported() {
51-
getAndroidComponentXmlElement().isExported()
52-
or
53-
not getAndroidComponentXmlElement().isNotExported() and hasIntentFilter()
54-
}
5553
}
5654

5755
/** An Android broadcast receiver. */
58-
class AndroidBroadcastReceiver extends AndroidComponent {
56+
class AndroidBroadcastReceiver extends ExportableAndroidComponent {
5957
AndroidBroadcastReceiver() {
6058
this.getASupertype*().hasQualifiedName("android.content", "BroadcastReceiver")
6159
}
62-
63-
/** Holds if this Android component is configured as `exported` or has intent filters configured without `exported` explicitly disabled in an `AndroidManifest.xml` file. */
64-
override predicate isExported() {
65-
getAndroidComponentXmlElement().isExported()
66-
or
67-
not getAndroidComponentXmlElement().isNotExported() and hasIntentFilter()
68-
}
6960
}
7061

7162
/** An Android content provider. */
72-
class AndroidContentProvider extends AndroidComponent {
63+
class AndroidContentProvider extends ExportableAndroidComponent {
7364
AndroidContentProvider() {
7465
this.getASupertype*().hasQualifiedName("android.content", "ContentProvider")
7566
}

‎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+
}

‎java/ql/test/experimental/query-tests/security/CWE-749/UnsafeActivity1.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 UnsafeActivity1 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/UnsafeActivity2.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 UnsafeActivity2 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)