Skip to content

Commit abeefca

Browse files
smowtonsmowton
authored
Merge pull request github#4947 from porcupineyhairs/DexLoading
Java : add query to detect insecure loading of Dex File
2 parents 7d2a60e + 11bf982 commit abeefca

File tree

5 files changed

+217
-0
lines changed

5 files changed

+217
-0
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qhelp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
It is dangerous to load Dex libraries from shared world-writable storage spaces. A malicious actor can replace a dex file with a maliciously crafted file
6+
which when loaded by the app can lead to code execution.
7+
</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>
12+
Loading a file from private storage instead of a world-writable one can prevent this issue,
13+
because the attacker cannot access files stored there.
14+
</p>
15+
</recommendation>
16+
17+
<example>
18+
<p>
19+
The following example loads a Dex file from a shared world-writable location. in this case,
20+
since the `/sdcard` directory is on external storage, anyone can read/write to the location.
21+
bypassing all Android security policies. Hence, this is insecure.
22+
</p>
23+
<sample src="InsecureDexLoadingBad.java" />
24+
25+
<p>
26+
The next example loads a Dex file stored inside the app's private storage.
27+
This is not exploitable as nobody else except the app can access the data stored there.
28+
</p>
29+
<sample src="InsecureDexLoadingGood.java" />
30+
</example>
31+
32+
<references>
33+
<li>
34+
Android Documentation:
35+
<a href="https://developer.android.com/training/data-storage/">Data and file storage overview</a>.
36+
</li>
37+
<li>
38+
Android Documentation:
39+
<a href="https://developer.android.com/reference/dalvik/system/DexClassLoader">DexClassLoader</a>.
40+
</li>
41+
</references>
42+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.ql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name Insecure loading of an Android Dex File
3+
* @description Loading a DEX library located in a world-writable location such as
4+
* an SD card can lead to arbitrary code execution vulnerabilities.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/android-insecure-dex-loading
9+
* @tags security
10+
* external/cwe/cwe-094
11+
*/
12+
13+
import java
14+
import InsecureDexLoading
15+
import DataFlow::PathGraph
16+
17+
from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureDexConfiguration conf
18+
where conf.hasFlowPath(source, sink)
19+
select sink.getNode(), source, sink, "Potential arbitrary code execution due to $@.",
20+
source.getNode(), "a value loaded from a world-writable source."

‎java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qll

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
4+
/**
5+
* A taint-tracking configuration detecting unsafe use of a
6+
* `DexClassLoader` by an Android app.
7+
*/
8+
class InsecureDexConfiguration extends TaintTracking::Configuration {
9+
InsecureDexConfiguration() { this = "Insecure Dex File Load" }
10+
11+
override predicate isSource(DataFlow::Node source) { source instanceof InsecureDexSource }
12+
13+
override predicate isSink(DataFlow::Node sink) { sink instanceof InsecureDexSink }
14+
15+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
16+
flowStep(pred, succ)
17+
}
18+
}
19+
20+
/** A data flow source for insecure Dex class loading vulnerabilities. */
21+
abstract class InsecureDexSource extends DataFlow::Node { }
22+
23+
/** A data flow sink for insecure Dex class loading vulnerabilities. */
24+
abstract class InsecureDexSink extends DataFlow::Node { }
25+
26+
private predicate flowStep(DataFlow::Node pred, DataFlow::Node succ) {
27+
// propagate from a `java.io.File` via the `File.getAbsolutePath` call.
28+
exists(MethodAccess m |
29+
m.getMethod().getDeclaringType() instanceof TypeFile and
30+
m.getMethod().hasName("getAbsolutePath") and
31+
m.getQualifier() = pred.asExpr() and
32+
m = succ.asExpr()
33+
)
34+
or
35+
// propagate from a `java.io.File` via the `File.toString` call.
36+
exists(MethodAccess m |
37+
m.getMethod().getDeclaringType() instanceof TypeFile and
38+
m.getMethod().hasName("toString") and
39+
m.getQualifier() = pred.asExpr() and
40+
m = succ.asExpr()
41+
)
42+
or
43+
// propagate to newly created `File` if the parent directory of the new `File` is tainted
44+
exists(ConstructorCall cc |
45+
cc.getConstructedType() instanceof TypeFile and
46+
cc.getArgument(0) = pred.asExpr() and
47+
cc = succ.asExpr()
48+
)
49+
}
50+
51+
/**
52+
* An argument to a `DexClassLoader` call taken as a sink for
53+
* insecure Dex class loading vulnerabilities.
54+
*/
55+
private class DexClassLoader extends InsecureDexSink {
56+
DexClassLoader() {
57+
exists(ConstructorCall cc |
58+
cc.getConstructedType().hasQualifiedName("dalvik.system", "DexClassLoader")
59+
|
60+
this.asExpr() = cc.getArgument(0)
61+
)
62+
}
63+
}
64+
65+
/**
66+
* A `File` instance which reads from an SD card
67+
* taken as a source for insecure Dex class loading vulnerabilities.
68+
*/
69+
private class ExternalFile extends InsecureDexSource {
70+
ExternalFile() {
71+
exists(ConstructorCall cc, Argument a |
72+
cc.getConstructedType() instanceof TypeFile and
73+
a = cc.getArgument(0) and
74+
a.(CompileTimeConstantExpr).getStringValue().matches("%sdcard%")
75+
|
76+
this.asExpr() = a
77+
)
78+
}
79+
}
80+
81+
/**
82+
* A directory or file which may be stored in an world writable directory
83+
* taken as a source for insecure Dex class loading vulnerabilities.
84+
*/
85+
private class ExternalStorageDirSource extends InsecureDexSource {
86+
ExternalStorageDirSource() {
87+
exists(Method m |
88+
m.getDeclaringType().hasQualifiedName("android.os", "Environment") and
89+
m.hasName("getExternalStorageDirectory")
90+
or
91+
m.getDeclaringType().hasQualifiedName("android.content", "Context") and
92+
m.hasName([
93+
"getExternalFilesDir", "getExternalFilesDirs", "getExternalMediaDirs",
94+
"getExternalCacheDir", "getExternalCacheDirs"
95+
])
96+
|
97+
this.asExpr() = m.getAReference()
98+
)
99+
}
100+
}

‎java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingBad.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
2+
import android.app.Application;
3+
import android.content.Context;
4+
import android.content.pm.PackageInfo;
5+
import android.os.Bundle;
6+
7+
import dalvik.system.DexClassLoader;
8+
import dalvik.system.DexFile;
9+
10+
public class InsecureDexLoading extends Application {
11+
@Override
12+
public void onCreate() {
13+
super.onCreate();
14+
updateChecker();
15+
}
16+
17+
private void updateChecker() {
18+
try {
19+
File file = new File("/sdcard/updater.apk");
20+
if (file.exists() && file.isFile() && file.length() <= 1000) {
21+
DexClassLoader cl = new DexClassLoader(file.getAbsolutePath(), getCacheDir().getAbsolutePath(), null,
22+
getClassLoader());
23+
int version = (int) cl.loadClass("my.package.class").getDeclaredMethod("myMethod").invoke(null);
24+
if (Build.VERSION.SDK_INT < version) {
25+
Toast.makeText(this, "Loaded Dex!", Toast.LENGTH_LONG).show();
26+
}
27+
}
28+
} catch (Exception e) {
29+
// ignore
30+
}
31+
}
32+
}

‎java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingGood.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
public class SecureDexLoading extends Application {
2+
@Override
3+
public void onCreate() {
4+
super.onCreate();
5+
updateChecker();
6+
}
7+
8+
private void updateChecker() {
9+
try {
10+
File file = new File(getCacheDir() + "/updater.apk");
11+
if (file.exists() && file.isFile() && file.length() <= 1000) {
12+
DexClassLoader cl = new DexClassLoader(file.getAbsolutePath(), getCacheDir().getAbsolutePath(), null,
13+
getClassLoader());
14+
int version = (int) cl.loadClass("my.package.class").getDeclaredMethod("myMethod").invoke(null);
15+
if (Build.VERSION.SDK_INT < version) {
16+
Toast.makeText(this, "Securely loaded Dex!", Toast.LENGTH_LONG).show();
17+
}
18+
}
19+
} catch (Exception e) {
20+
// ignore
21+
}
22+
}
23+
}

0 commit comments

Comments
 (0)