diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qhelp new file mode 100644 index 00000000000..216bdeaebf3 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qhelp @@ -0,0 +1,42 @@ + + + +

+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 +which when loaded by the app can lead to code execution. +

+
+ + +

+ Loading a file from private storage instead of a world-writable one can prevent this issue, + because the attacker cannot access files stored there. +

+
+ + +

+ The following example loads a Dex file from a shared world-writable location. in this case, + since the `/sdcard` directory is on external storage, anyone can read/write to the location. + bypassing all Android security policies. Hence, this is insecure. +

+ + +

+ The next example loads a Dex file stored inside the app's private storage. + This is not exploitable as nobody else except the app can access the data stored there. +

+ +
+ + +
  • + Android Documentation: + Data and file storage overview. +
  • +
  • + Android Documentation: + DexClassLoader. +
  • +
    +
    diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.ql b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.ql new file mode 100644 index 00000000000..bae3ed63d70 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.ql @@ -0,0 +1,20 @@ +/** + * @name Insecure loading of an Android Dex File + * @description Loading a DEX library located in a world-writable location such as + * an SD card can lead to arbitrary code execution vulnerabilities. + * @kind path-problem + * @problem.severity error + * @precision high + * @id java/android-insecure-dex-loading + * @tags security + * external/cwe/cwe-094 + */ + +import java +import InsecureDexLoading +import DataFlow::PathGraph + +from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureDexConfiguration conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Potential arbitrary code execution due to $@.", + source.getNode(), "a value loaded from a world-writable source." diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qll b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qll new file mode 100644 index 00000000000..0ee0954216e --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qll @@ -0,0 +1,100 @@ +import java +import semmle.code.java.dataflow.FlowSources + +/** + * A taint-tracking configuration detecting unsafe use of a + * `DexClassLoader` by an Android app. + */ +class InsecureDexConfiguration extends TaintTracking::Configuration { + InsecureDexConfiguration() { this = "Insecure Dex File Load" } + + override predicate isSource(DataFlow::Node source) { source instanceof InsecureDexSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof InsecureDexSink } + + override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + flowStep(pred, succ) + } +} + +/** A data flow source for insecure Dex class loading vulnerabilities. */ +abstract class InsecureDexSource extends DataFlow::Node { } + +/** A data flow sink for insecure Dex class loading vulnerabilities. */ +abstract class InsecureDexSink extends DataFlow::Node { } + +private predicate flowStep(DataFlow::Node pred, DataFlow::Node succ) { + // propagate from a `java.io.File` via the `File.getAbsolutePath` call. + exists(MethodAccess m | + m.getMethod().getDeclaringType() instanceof TypeFile and + m.getMethod().hasName("getAbsolutePath") and + m.getQualifier() = pred.asExpr() and + m = succ.asExpr() + ) + or + // propagate from a `java.io.File` via the `File.toString` call. + exists(MethodAccess m | + m.getMethod().getDeclaringType() instanceof TypeFile and + m.getMethod().hasName("toString") and + m.getQualifier() = pred.asExpr() and + m = succ.asExpr() + ) + or + // propagate to newly created `File` if the parent directory of the new `File` is tainted + exists(ConstructorCall cc | + cc.getConstructedType() instanceof TypeFile and + cc.getArgument(0) = pred.asExpr() and + cc = succ.asExpr() + ) +} + +/** + * An argument to a `DexClassLoader` call taken as a sink for + * insecure Dex class loading vulnerabilities. + */ +private class DexClassLoader extends InsecureDexSink { + DexClassLoader() { + exists(ConstructorCall cc | + cc.getConstructedType().hasQualifiedName("dalvik.system", "DexClassLoader") + | + this.asExpr() = cc.getArgument(0) + ) + } +} + +/** + * A `File` instance which reads from an SD card + * taken as a source for insecure Dex class loading vulnerabilities. + */ +private class ExternalFile extends InsecureDexSource { + ExternalFile() { + exists(ConstructorCall cc, Argument a | + cc.getConstructedType() instanceof TypeFile and + a = cc.getArgument(0) and + a.(CompileTimeConstantExpr).getStringValue().matches("%sdcard%") + | + this.asExpr() = a + ) + } +} + +/** + * A directory or file which may be stored in an world writable directory + * taken as a source for insecure Dex class loading vulnerabilities. + */ +private class ExternalStorageDirSource extends InsecureDexSource { + ExternalStorageDirSource() { + exists(Method m | + m.getDeclaringType().hasQualifiedName("android.os", "Environment") and + m.hasName("getExternalStorageDirectory") + or + m.getDeclaringType().hasQualifiedName("android.content", "Context") and + m.hasName([ + "getExternalFilesDir", "getExternalFilesDirs", "getExternalMediaDirs", + "getExternalCacheDir", "getExternalCacheDirs" + ]) + | + this.asExpr() = m.getAReference() + ) + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingBad.java b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingBad.java new file mode 100644 index 00000000000..869b6bc571c --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingBad.java @@ -0,0 +1,32 @@ + +import android.app.Application; +import android.content.Context; +import android.content.pm.PackageInfo; +import android.os.Bundle; + +import dalvik.system.DexClassLoader; +import dalvik.system.DexFile; + +public class InsecureDexLoading extends Application { + @Override + public void onCreate() { + super.onCreate(); + updateChecker(); + } + + private void updateChecker() { + try { + File file = new File("/sdcard/updater.apk"); + if (file.exists() && file.isFile() && file.length() <= 1000) { + DexClassLoader cl = new DexClassLoader(file.getAbsolutePath(), getCacheDir().getAbsolutePath(), null, + getClassLoader()); + int version = (int) cl.loadClass("my.package.class").getDeclaredMethod("myMethod").invoke(null); + if (Build.VERSION.SDK_INT < version) { + Toast.makeText(this, "Loaded Dex!", Toast.LENGTH_LONG).show(); + } + } + } catch (Exception e) { + // ignore + } + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingGood.java b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingGood.java new file mode 100644 index 00000000000..e45e3938f7b --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingGood.java @@ -0,0 +1,23 @@ +public class SecureDexLoading extends Application { + @Override + public void onCreate() { + super.onCreate(); + updateChecker(); + } + + private void updateChecker() { + try { + File file = new File(getCacheDir() + "/updater.apk"); + if (file.exists() && file.isFile() && file.length() <= 1000) { + DexClassLoader cl = new DexClassLoader(file.getAbsolutePath(), getCacheDir().getAbsolutePath(), null, + getClassLoader()); + int version = (int) cl.loadClass("my.package.class").getDeclaredMethod("myMethod").invoke(null); + if (Build.VERSION.SDK_INT < version) { + Toast.makeText(this, "Securely loaded Dex!", Toast.LENGTH_LONG).show(); + } + } + } catch (Exception e) { + // ignore + } + } +} \ No newline at end of file