Skip to content

Commit 8687c5c

Browse files
porcupineyhairssmowton
andauthored
Apply suggestions from code review
Co-authored-by: Chris Smowton <[email protected]>
1 parent 2ca9516 commit 8687c5c

File tree

4 files changed

+14
-14
lines changed

4 files changed

+14
-14
lines changed

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,29 @@
22
<qhelp>
33
<overview>
44
<p>
5-
Shared world writable storage spaces are not secure to load Dex libraries from. A malicious actor can replace a dex file with a maliciously crafted file
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
66
which when loaded by the app can lead to code execution.
77
</p>
88
</overview>
99

1010
<recommendation>
1111
<p>
12-
Loading a file from private storage instead of a world writable one can prevent this issue.
13-
As the attacker cannot access files stored by the app in its private storage.
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.
1414
</p>
1515
</recommendation>
1616

1717
<example>
1818
<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, any one can read/write to the location.
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.
2121
bypassing all Android security policies. Hence, this is insecure.
2222
</p>
2323
<sample src="InsecureDexLoadingBad.java" />
2424

2525
<p>
2626
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 here.
27+
This is not exploitable as nobody else except the app can access the data stored there.
2828
</p>
2929
<sample src="InsecureDexLoadingGood.java" />
3030
</example>

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @name Insecure loading of an Android Dex File
3-
* @description Loading a DEX library located in a world-readable/ writable location such as
4-
* a SD card can cause arbitary code execution vulnerabilities.
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.
55
* @kind path-problem
66
* @problem.severity error
77
* @precision high
@@ -16,5 +16,5 @@ import DataFlow::PathGraph
1616

1717
from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureDexConfiguration conf
1818
where conf.hasFlowPath(source, sink)
19-
select sink.getNode(), source, sink, "Potential arbitary code execution due to $@.",
20-
source.getNode(), "a value loaded from a world readable/writable source."
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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import java
22
import semmle.code.java.dataflow.FlowSources
33

44
/**
5-
* A taint-tracking configuration fordetecting unsafe use of a
5+
* A taint-tracking configuration detecting unsafe use of a
66
* `DexClassLoader` by an Android app.
77
*/
88
class InsecureDexConfiguration extends TaintTracking::Configuration {
@@ -63,7 +63,7 @@ private class DexClassLoader extends InsecureDexSink {
6363
}
6464

6565
/**
66-
* An `File` instance which reads from an SD card
66+
* A `File` instance which reads from an SD card
6767
* taken as a source for insecure Dex class loading vulnerabilities.
6868
*/
6969
private class ExternalFile extends InsecureDexSource {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ private void updateChecker() {
2222
getClassLoader());
2323
int version = (int) cl.loadClass("my.package.class").getDeclaredMethod("myMethod").invoke(null);
2424
if (Build.VERSION.SDK_INT < version) {
25-
Toast.makeText(this, "Securely loaded Dex!", Toast.LENGTH_LONG).show();
25+
Toast.makeText(this, "Loaded Dex!", Toast.LENGTH_LONG).show();
2626
}
2727
}
2828
} catch (Exception e) {
2929
// ignore
3030
}
3131
}
32-
}
32+
}

0 commit comments

Comments
 (0)