Skip to content

Commit 028a72b

Browse files
aschackmullaschackmull
authored
Merge pull request #4610 from luchua-bc/java-nfe-local-android-dos
Java: Query to detect Local Android DoS caused by NFE
2 parents 2fa9037 + 018d5c4 commit 028a72b

File tree

10 files changed

+269
-0
lines changed

10 files changed

+269
-0
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-755/NFEAndroidDoS.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
public class NFEAndroidDoS extends Activity {
2+
public void onCreate(Bundle savedInstanceState) {
3+
super.onCreate(savedInstanceState);
4+
setContentView(R.layout.activity_view);
5+
6+
// BAD: Uncaught NumberFormatException due to remote user inputs
7+
{
8+
String minPriceStr = getIntent().getStringExtra("priceMin");
9+
double minPrice = Double.parseDouble(minPriceStr);
10+
}
11+
12+
// GOOD: Use the proper Android method to get number extra
13+
{
14+
int width = getIntent().getIntExtra("width", 0);
15+
int height = getIntent().getIntExtra("height", 0);
16+
}
17+
}
18+
}

‎java/ql/src/experimental/Security/CWE/CWE-755/NFEAndroidDoS.qhelp‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>NumberFormatException (NFE) thrown but not caught by an Android application will crash the application. If the application allows external inputs, an attacker can send an invalid number as intent extra to trigger NFE, which introduces local Denial of Service (Dos) attack.</p>
6+
<p>
7+
This is a common problem in Android development since Android components don't have
8+
<code>throw Exception(...)</code>
9+
in their class and method definitions.
10+
</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>
15+
Use the Android methods intended to get number extras e.g.
16+
<code>Intent.getFloatExtra(String name, float defaultValue)</code>
17+
since they have the built-in try/catch processing, or explicitly do try/catch in the application.
18+
</p>
19+
</recommendation>
20+
21+
<example>
22+
<p>The following example shows both 'BAD' and 'GOOD' configurations. In the 'BAD' configuration, number value is retrieved as string extra then parsed to double. In the 'GOOD' configuration, number value is retrieved as integer extra.</p>
23+
<sample src="NFEAndroidDoS.java" />
24+
</example>
25+
26+
<references>
27+
<li>
28+
CWE:
29+
<a href="https://cwe.mitre.org/data/definitions/749.html">CWE-755: Improper Handling of Exceptional Conditions</a>
30+
</li>
31+
<li>
32+
Android Developers:
33+
<a href="https://developer.android.com/topic/performance/vitals/crash">Android Crashes</a>
34+
</li>
35+
<li>
36+
Google Analytics:
37+
<a href="https://developers.google.com/analytics/devguides/collection/android/v4/exceptions">Crash and Exception Measurement Using the Google Analytics SDK</a>
38+
</li>
39+
</references>
40+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-755/NFEAndroidDoS.ql‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* @name Local Android DoS Caused By NumberFormatException
3+
* @id java/android/nfe-local-android-dos
4+
* @description NumberFormatException thrown but not caught by an Android application that allows external inputs can crash the application, constituting a local Denial of Service (DoS) attack.
5+
* @kind path-problem
6+
* @tags security
7+
* external/cwe/cwe-755
8+
*/
9+
10+
import java
11+
import semmle.code.java.frameworks.android.Intent
12+
import semmle.code.java.dataflow.FlowSources
13+
import semmle.code.java.NumberFormatException
14+
import DataFlow::PathGraph
15+
16+
/**
17+
* Taint configuration tracking flow from untrusted inputs to number conversion calls in exported Android compononents.
18+
*/
19+
class NFELocalDoSConfiguration extends TaintTracking::Configuration {
20+
NFELocalDoSConfiguration() { this = "NFELocalDoSConfiguration" }
21+
22+
/** Holds if source is a remote flow source */
23+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
24+
25+
/** Holds if NFE is thrown but not caught */
26+
override predicate isSink(DataFlow::Node sink) {
27+
exists(Expr e |
28+
e.getEnclosingCallable().getDeclaringType().(ExportableAndroidComponent).isExported() and
29+
throwsNFE(e) and
30+
not exists(TryStmt t |
31+
t.getBlock() = e.getAnEnclosingStmt() and
32+
catchesNFE(t)
33+
) and
34+
sink.asExpr() = e
35+
)
36+
}
37+
}
38+
39+
from DataFlow::PathNode source, DataFlow::PathNode sink, NFELocalDoSConfiguration conf
40+
where conf.hasFlowPath(source, sink)
41+
select sink.getNode(), source, sink,
42+
"Uncaught NumberFormatException in an exported Android component due to $@.", source.getNode(),
43+
"user-provided value"

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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=".NFEAndroidDoS"
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=".SafeActivity" />
24+
</application>
25+
26+
</manifest>

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.example.app;
2+
3+
import android.app.Activity;
4+
5+
import android.os.Bundle;
6+
7+
8+
/** A utility program for getting intent extra information from Android activity */
9+
public class IntentUtils {
10+
11+
/** Get double extra */
12+
public static double getDoubleExtra(Activity a, String key) {
13+
String value = a.getIntent().getStringExtra(key);
14+
return Double.parseDouble(value);
15+
}
16+
}

‎java/ql/test/experimental/query-tests/security/CWE-755/NFEAndroidDoS.expected‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
edges
2+
| NFEAndroidDoS.java:13:24:13:34 | getIntent(...) : Intent | NFEAndroidDoS.java:14:21:14:51 | parseDouble(...) |
3+
| NFEAndroidDoS.java:22:21:22:31 | getIntent(...) : Intent | NFEAndroidDoS.java:23:15:23:40 | parseInt(...) |
4+
| NFEAndroidDoS.java:25:22:25:32 | getIntent(...) : Intent | NFEAndroidDoS.java:26:16:26:42 | parseInt(...) |
5+
| NFEAndroidDoS.java:43:24:43:34 | getIntent(...) : Intent | NFEAndroidDoS.java:44:21:44:43 | new Double(...) |
6+
| NFEAndroidDoS.java:43:24:43:34 | getIntent(...) : Intent | NFEAndroidDoS.java:47:21:47:47 | valueOf(...) |
7+
nodes
8+
| NFEAndroidDoS.java:13:24:13:34 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent |
9+
| NFEAndroidDoS.java:14:21:14:51 | parseDouble(...) | semmle.label | parseDouble(...) |
10+
| NFEAndroidDoS.java:22:21:22:31 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent |
11+
| NFEAndroidDoS.java:23:15:23:40 | parseInt(...) | semmle.label | parseInt(...) |
12+
| NFEAndroidDoS.java:25:22:25:32 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent |
13+
| NFEAndroidDoS.java:26:16:26:42 | parseInt(...) | semmle.label | parseInt(...) |
14+
| NFEAndroidDoS.java:43:24:43:34 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent |
15+
| NFEAndroidDoS.java:44:21:44:43 | new Double(...) | semmle.label | new Double(...) |
16+
| NFEAndroidDoS.java:47:21:47:47 | valueOf(...) | semmle.label | valueOf(...) |
17+
#select
18+
| NFEAndroidDoS.java:14:21:14:51 | parseDouble(...) | NFEAndroidDoS.java:13:24:13:34 | getIntent(...) : Intent | NFEAndroidDoS.java:14:21:14:51 | parseDouble(...) | Uncaught NumberFormatException in an exported Android component due to $@. | NFEAndroidDoS.java:13:24:13:34 | getIntent(...) | user-provided value |
19+
| NFEAndroidDoS.java:23:15:23:40 | parseInt(...) | NFEAndroidDoS.java:22:21:22:31 | getIntent(...) : Intent | NFEAndroidDoS.java:23:15:23:40 | parseInt(...) | Uncaught NumberFormatException in an exported Android component due to $@. | NFEAndroidDoS.java:22:21:22:31 | getIntent(...) | user-provided value |
20+
| NFEAndroidDoS.java:26:16:26:42 | parseInt(...) | NFEAndroidDoS.java:25:22:25:32 | getIntent(...) : Intent | NFEAndroidDoS.java:26:16:26:42 | parseInt(...) | Uncaught NumberFormatException in an exported Android component due to $@. | NFEAndroidDoS.java:25:22:25:32 | getIntent(...) | user-provided value |
21+
| NFEAndroidDoS.java:44:21:44:43 | new Double(...) | NFEAndroidDoS.java:43:24:43:34 | getIntent(...) : Intent | NFEAndroidDoS.java:44:21:44:43 | new Double(...) | Uncaught NumberFormatException in an exported Android component due to $@. | NFEAndroidDoS.java:43:24:43:34 | getIntent(...) | user-provided value |
22+
| NFEAndroidDoS.java:47:21:47:47 | valueOf(...) | NFEAndroidDoS.java:43:24:43:34 | getIntent(...) : Intent | NFEAndroidDoS.java:47:21:47:47 | valueOf(...) | Uncaught NumberFormatException in an exported Android component due to $@. | NFEAndroidDoS.java:43:24:43:34 | getIntent(...) | user-provided value |

‎java/ql/test/experimental/query-tests/security/CWE-755/NFEAndroidDoS.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package com.example.app;
2+
3+
import android.app.Activity;
4+
import android.os.Bundle;
5+
6+
/** Android activity that tests app crash by NumberFormatException */
7+
public class NFEAndroidDoS extends Activity {
8+
// BAD - parse string extra to double
9+
public void testOnCreate1(Bundle savedInstanceState) {
10+
super.onCreate(savedInstanceState);
11+
setContentView(-1);
12+
13+
String minPriceStr = getIntent().getStringExtra("priceMin");
14+
double minPrice = Double.parseDouble(minPriceStr);
15+
}
16+
17+
// BAD - parse string extra to integer
18+
public void testOnCreate2(Bundle savedInstanceState) {
19+
super.onCreate(savedInstanceState);
20+
setContentView(-1);
21+
22+
String widthStr = getIntent().getStringExtra("width");
23+
int width = Integer.parseInt(widthStr);
24+
25+
String heightStr = getIntent().getStringExtra("height");
26+
int height = Integer.parseInt(heightStr);
27+
}
28+
29+
// GOOD - parse int extra to integer
30+
public void testOnCreate3(Bundle savedInstanceState) {
31+
super.onCreate(savedInstanceState);
32+
setContentView(-1);
33+
34+
int width = getIntent().getIntExtra("width", 0);
35+
int height = getIntent().getIntExtra("height", 0);
36+
}
37+
38+
// BAD - convert string extra to double
39+
public void testOnCreate4(Bundle savedInstanceState) {
40+
super.onCreate(savedInstanceState);
41+
setContentView(-1);
42+
43+
String minPriceStr = getIntent().getStringExtra("priceMin");
44+
double minPrice = new Double(minPriceStr);
45+
46+
String maxPriceStr = getIntent().getStringExtra("priceMax");
47+
double maxPrice = Double.valueOf(minPriceStr);
48+
}
49+
50+
// GOOD - parse string extra to double with caught NFE
51+
public void testOnCreate5(Bundle savedInstanceState) {
52+
super.onCreate(savedInstanceState);
53+
setContentView(-1);
54+
55+
double minPrice = 0;
56+
try {
57+
String minPriceStr = getIntent().getStringExtra("priceMin");
58+
minPrice = Double.parseDouble(minPriceStr);
59+
} catch (NumberFormatException nfe) {
60+
nfe.printStackTrace();
61+
}
62+
}
63+
64+
// GOOD - parse string extra to double with caught NFE as the supertype Throwable
65+
public void testOnCreate6(Bundle savedInstanceState) {
66+
super.onCreate(savedInstanceState);
67+
setContentView(-1);
68+
69+
double minPrice = 0;
70+
try {
71+
String minPriceStr = getIntent().getStringExtra("priceMin");
72+
minPrice = Double.parseDouble(minPriceStr);
73+
} catch (Throwable te) {
74+
te.printStackTrace();
75+
}
76+
}
77+
78+
// BAD - parse string extra to double
79+
// Note this case of invoking utility method that takes an Activity a then calls `a.getIntent().getStringExtra(...)` is not yet detected thus is beyond what the query is capable of.
80+
public void testOnCreate7(Bundle savedInstanceState) {
81+
super.onCreate(savedInstanceState);
82+
setContentView(-1);
83+
84+
double priceMin = IntentUtils.getDoubleExtra(this, "priceMin");
85+
}
86+
}

‎java/ql/test/experimental/query-tests/security/CWE-755/NFEAndroidDoS.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-755/NFEAndroidDoS.ql

‎java/ql/test/experimental/query-tests/security/CWE-755/SafeActivity.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.example.app;
2+
3+
import android.app.Activity;
4+
import android.os.Bundle;
5+
6+
/** Android activity that tests app crash by NumberFormatException, which is not exported in `AndroidManifest.xml` */
7+
public class SafeActivity extends Activity {
8+
// BAD - parse string extra to double
9+
public void testOnCreate1(Bundle savedInstanceState) {
10+
super.onCreate(savedInstanceState);
11+
setContentView(-1);
12+
13+
String minPriceStr = getIntent().getStringExtra("priceMin");
14+
double minPrice = Double.parseDouble(minPriceStr);
15+
}
16+
}

‎java/ql/test/experimental/query-tests/security/CWE-755/options‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0

0 commit comments

Comments
 (0)