Skip to content

Commit 76a0db8

Browse files
luchua-bcluchua-bc
committed
Query for detecting Local Android DoS caused by NFE
1 parent 31ec798 commit 76a0db8

File tree

8 files changed

+262
-0
lines changed

8 files changed

+262
-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: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
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, which is 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.frameworks.android.WebView
13+
import semmle.code.java.dataflow.FlowSources
14+
import DataFlow::PathGraph
15+
16+
/** Code from java/ql/src/Violations of Best Practice/Exception Handling/NumberFormatException.ql */
17+
private class SpecialMethodAccess extends MethodAccess {
18+
predicate isValueOfMethod(string klass) {
19+
this.getMethod().getName() = "valueOf" and
20+
this.getQualifier().getType().(RefType).hasQualifiedName("java.lang", klass) and
21+
this.getAnArgument().getType().(RefType).hasQualifiedName("java.lang", "String")
22+
}
23+
24+
predicate isParseMethod(string klass, string name) {
25+
this.getMethod().getName() = name and
26+
this.getQualifier().getType().(RefType).hasQualifiedName("java.lang", klass)
27+
}
28+
29+
predicate throwsNFE() {
30+
this.isParseMethod("Byte", "parseByte") or
31+
this.isParseMethod("Short", "parseShort") or
32+
this.isParseMethod("Integer", "parseInt") or
33+
this.isParseMethod("Long", "parseLong") or
34+
this.isParseMethod("Float", "parseFloat") or
35+
this.isParseMethod("Double", "parseDouble") or
36+
this.isParseMethod("Byte", "decode") or
37+
this.isParseMethod("Short", "decode") or
38+
this.isParseMethod("Integer", "decode") or
39+
this.isParseMethod("Long", "decode") or
40+
this.isValueOfMethod("Byte") or
41+
this.isValueOfMethod("Short") or
42+
this.isValueOfMethod("Integer") or
43+
this.isValueOfMethod("Long") or
44+
this.isValueOfMethod("Float") or
45+
this.isValueOfMethod("Double")
46+
}
47+
}
48+
49+
private class SpecialClassInstanceExpr extends ClassInstanceExpr {
50+
predicate isStringConstructor(string klass) {
51+
this.getType().(RefType).hasQualifiedName("java.lang", klass) and
52+
this.getAnArgument().getType().(RefType).hasQualifiedName("java.lang", "String") and
53+
this.getNumArgument() = 1
54+
}
55+
56+
predicate throwsNFE() {
57+
this.isStringConstructor("Byte") or
58+
this.isStringConstructor("Short") or
59+
this.isStringConstructor("Integer") or
60+
this.isStringConstructor("Long") or
61+
this.isStringConstructor("Float") or
62+
this.isStringConstructor("Double")
63+
}
64+
}
65+
66+
class NumberFormatException extends RefType {
67+
NumberFormatException() { this.hasQualifiedName("java.lang", "NumberFormatException") }
68+
}
69+
70+
private predicate catchesNFE(TryStmt t) {
71+
exists(CatchClause cc, LocalVariableDeclExpr v |
72+
t.getACatchClause() = cc and
73+
cc.getVariable() = v and
74+
v.getType().(RefType).getASubtype*() instanceof NumberFormatException
75+
)
76+
}
77+
78+
private predicate throwsNFE(Expr e) {
79+
e.(SpecialClassInstanceExpr).throwsNFE() or e.(SpecialMethodAccess).throwsNFE()
80+
}
81+
82+
/**
83+
* Taint configuration tracking flow from untrusted inputs to number conversion calls.
84+
*/
85+
class NFELocalDoSConfiguration extends TaintTracking::Configuration {
86+
NFELocalDoSConfiguration() { this = "NFELocalDoSConfiguration" }
87+
88+
/** Holds if source is a remote flow source */
89+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
90+
91+
/** Holds if NFE is thrown but not caught */
92+
override predicate isSink(DataFlow::Node sink) {
93+
exists(Expr e |
94+
throwsNFE(e) and
95+
not exists(TryStmt t |
96+
t.getBlock() = e.getEnclosingStmt().getEnclosingStmt*() and
97+
catchesNFE(t)
98+
) and
99+
sink.asExpr() = e
100+
)
101+
}
102+
}
103+
104+
from DataFlow::PathNode source, DataFlow::PathNode sink, NFELocalDoSConfiguration conf
105+
where conf.hasFlowPath(source, sink)
106+
select sink.getNode(), source, sink, "Local Android Denial of Service due to $@.", source.getNode(),
107+
"user-provided value"

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
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+
</application>
23+
24+
</manifest>

‎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(...) | Local Android Denial of Service 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(...) | Local Android Denial of Service 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(...) | Local Android Denial of Service 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(...) | Local Android Denial of Service 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(...) | Local Android Denial of Service 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: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
}

‎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/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)