-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Java: Insecure Loading of Class in Android App without Package Signature Checking #14752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
atorralba
merged 17 commits into
github:main
from
masterofnow:LoadClassNoSignatureCheck
Dec 22, 2023
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
fd66f47
Added LoadClassNoSignatureCheck.ql
masterofnow 2059235
Updated text in LoadClassNoSignatureCheck.qhelp
masterofnow 532f6a5
Removed @kind path-problem in comment. Added text message in select.
masterofnow 7d774f1
Merge branch 'main' into LoadClassNoSignatureCheck
masterofnow 2952d8f
Updated query to cover broader detection.
masterofnow 57d897d
Merge branch 'main' into LoadClassNoSignatureCheck
masterofnow 8538c12
Merge branch 'github:main' into LoadClassNoSignatureCheck
masterofnow e1b8fab
Use global instead of local taint tracking.
masterofnow 99b273d
Apply suggestions from code review
masterofnow 4a77f45
Minor adjustment to resolve error for codeql version 2.15.4
masterofnow e85c4b5
Update query from code review feedback to express it as a dataflow pr…
masterofnow 3970852
Minor fixes
atorralba 25c818f
Added unit test files.
masterofnow 8dc522f
Merge remote-tracking branch 'origin/LoadClassNoSignatureCheck' into …
masterofnow 7162540
Added options, .qhelp and .expected file for unit test.
masterofnow cb5733d
Apply suggestions from code review
masterofnow 0fd0975
Added sample java file for qhelp to render correctly.
masterofnow File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
27 changes: 27 additions & 0 deletions
27
java/ql/src/experimental/Security/CWE/CWE-470/BadClassLoader.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package poc.sample.classloader; | ||
|
||
import android.app.Application; | ||
import android.content.pm.PackageInfo; | ||
import android.content.Context; | ||
import android.util.Log; | ||
|
||
public class BadClassLoader extends Application { | ||
@Override | ||
public void onCreate() { | ||
super.onCreate(); | ||
for (PackageInfo p : getPackageManager().getInstalledPackages(0)) { | ||
try { | ||
if (p.packageName.startsWith("some.package.")) { | ||
Context appContext = createPackageContext(p.packageName, | ||
CONTEXT_INCLUDE_CODE | CONTEXT_IGNORE_SECURITY); | ||
ClassLoader classLoader = appContext.getClassLoader(); | ||
Object result = classLoader.loadClass("some.package.SomeClass") | ||
.getMethod("someMethod") | ||
.invoke(null); | ||
} | ||
} catch (Exception e) { | ||
Log.e("Class loading failed", e.toString()); | ||
} | ||
} | ||
} | ||
} |
31 changes: 31 additions & 0 deletions
31
java/ql/src/experimental/Security/CWE/CWE-470/GoodClassLoader.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package poc.sample.classloader; | ||
|
||
import android.app.Application; | ||
import android.content.pm.PackageInfo; | ||
import android.content.Context; | ||
import android.content.pm.PackageManager; | ||
import android.util.Log; | ||
|
||
public class GoodClassLoader extends Application { | ||
@Override | ||
public void onCreate() { | ||
super.onCreate(); | ||
PackageManager pm = getPackageManager(); | ||
for (PackageInfo p : pm.getInstalledPackages(0)) { | ||
try { | ||
if (p.packageName.startsWith("some.package.") && | ||
(pm.checkSignatures(p.packageName, getApplicationContext().getPackageName()) == PackageManager.SIGNATURE_MATCH) | ||
) { | ||
Context appContext = createPackageContext(p.packageName, | ||
CONTEXT_INCLUDE_CODE | CONTEXT_IGNORE_SECURITY); | ||
ClassLoader classLoader = appContext.getClassLoader(); | ||
Object result = classLoader.loadClass("some.package.SomeClass") | ||
.getMethod("someMethod") | ||
.invoke(null); | ||
} | ||
} catch (Exception e) { | ||
Log.e("Class loading failed", e.toString()); | ||
} | ||
} | ||
} | ||
} |
40 changes: 40 additions & 0 deletions
40
java/ql/src/experimental/Security/CWE/CWE-470/LoadClassNoSignatureCheck.qhelp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd"> | ||
<qhelp> | ||
|
||
<overview> | ||
<p> | ||
If an application loads classes or code from another app based solely on its package name without | ||
first checking its package signature, this could allow a malicious app with the same package name | ||
to be loaded through "package namespace squatting". | ||
If the victim user install such malicious app in the same device as the vulnerable app, the vulnerable app would load | ||
classes or code from the malicious app, potentially leading to arbitrary code execution. | ||
</p> | ||
</overview> | ||
|
||
<recommendation> | ||
<p> | ||
Verify the package signature in addition to the package name before loading any classes or code from another application. | ||
</p> | ||
</recommendation> | ||
|
||
<example> | ||
<p> | ||
The <code>BadClassLoader</code> class illustrates class loading with the <code>android.content.pm.PackageInfo.packageName.startsWith()</code> method without any check on the package signature. | ||
</p> | ||
<sample src="BadClassLoader.java" /> | ||
<p> | ||
The <code>GoodClassLoader</code> class illustrates class loading with correct package signature check using the <code>android.content.pm.PackageManager.checkSignatures()</code> method. | ||
</p> | ||
<sample src="GoodClassLoader.java" /> | ||
</example> | ||
|
||
|
||
<references> | ||
<li> | ||
<a href="https://blog.oversecured.com/Android-arbitrary-code-execution-via-third-party-package-contexts/"> | ||
Oversecured (Android: arbitrary code execution via third-party package contexts) | ||
</a> | ||
</li> | ||
</references> | ||
|
||
</qhelp> |
88 changes: 88 additions & 0 deletions
88
java/ql/src/experimental/Security/CWE/CWE-470/LoadClassNoSignatureCheck.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
/** | ||
* @name Load 3rd party classes or code ('unsafe reflection') without signature check | ||
* @description Loading classes or code from third-party packages without checking the | ||
* package signature could make the application | ||
* susceptible to package namespace squatting attacks, | ||
* potentially leading to arbitrary code execution. | ||
* @problem.severity error | ||
* @precision high | ||
* @kind path-problem | ||
* @id java/android/unsafe-reflection | ||
* @tags security | ||
* experimental | ||
* external/cwe/cwe-470 | ||
*/ | ||
|
||
import java | ||
import semmle.code.java.dataflow.TaintTracking | ||
import semmle.code.java.controlflow.Guards | ||
import semmle.code.java.dataflow.SSA | ||
import semmle.code.java.frameworks.android.Intent | ||
|
||
class CheckSignaturesGuard extends Guard instanceof EqualityTest { | ||
MethodCall checkSignatures; | ||
|
||
CheckSignaturesGuard() { | ||
this.getAnOperand() = checkSignatures and | ||
checkSignatures | ||
.getMethod() | ||
.hasQualifiedName("android.content.pm", "PackageManager", "checkSignatures") and | ||
exists(Expr signatureCheckResult | | ||
this.getAnOperand() = signatureCheckResult and signatureCheckResult != checkSignatures | ||
| | ||
signatureCheckResult.(CompileTimeConstantExpr).getIntValue() = 0 or | ||
signatureCheckResult | ||
.(FieldRead) | ||
.getField() | ||
.hasQualifiedName("android.content.pm", "PackageManager", "SIGNATURE_MATCH") | ||
) | ||
} | ||
|
||
Expr getCheckedExpr() { result = checkSignatures.getArgument(0) } | ||
} | ||
|
||
predicate signatureChecked(Expr safe) { | ||
exists(CheckSignaturesGuard g, SsaVariable v | | ||
v.getAUse() = g.getCheckedExpr() and | ||
safe = v.getAUse() and | ||
g.controls(safe.getBasicBlock(), g.(EqualityTest).polarity()) | ||
) | ||
} | ||
|
||
module InsecureLoadingConfig implements DataFlow::ConfigSig { | ||
predicate isSource(DataFlow::Node src) { | ||
exists(Method m | m = src.asExpr().(MethodCall).getMethod() | | ||
m.getDeclaringType().getASourceSupertype*() instanceof TypeContext and | ||
m.hasName("createPackageContext") and | ||
not signatureChecked(src.asExpr().(MethodCall).getArgument(0)) | ||
) | ||
} | ||
|
||
predicate isSink(DataFlow::Node sink) { | ||
exists(MethodCall ma | | ||
ma.getMethod().hasQualifiedName("java.lang", "ClassLoader", "loadClass") | ||
| | ||
sink.asExpr() = ma.getQualifier() | ||
) | ||
} | ||
|
||
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { | ||
exists(MethodCall ma, Method m | | ||
ma.getMethod() = m and | ||
m.getDeclaringType().getASourceSupertype*() instanceof TypeContext and | ||
m.hasName("getClassLoader") | ||
| | ||
node1.asExpr() = ma.getQualifier() and | ||
node2.asExpr() = ma | ||
) | ||
} | ||
} | ||
|
||
module InsecureLoadFlow = TaintTracking::Global<InsecureLoadingConfig>; | ||
|
||
import InsecureLoadFlow::PathGraph | ||
|
||
from InsecureLoadFlow::PathNode source, InsecureLoadFlow::PathNode sink | ||
where InsecureLoadFlow::flowPath(source, sink) | ||
select sink.getNode(), source, sink, "Class loaded from a $@ without signature check", | ||
source.getNode(), "third party library" |
27 changes: 27 additions & 0 deletions
27
java/ql/test/experimental/query-tests/security/CWE-470/BadClassLoader.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package poc.sample.classloader; | ||
|
||
import android.app.Application; | ||
import android.content.pm.PackageInfo; | ||
import android.content.Context; | ||
import android.util.Log; | ||
|
||
public class BadClassLoader extends Application { | ||
@Override | ||
public void onCreate() { | ||
super.onCreate(); | ||
for (PackageInfo p : getPackageManager().getInstalledPackages(0)) { | ||
try { | ||
if (p.packageName.startsWith("some.package.")) { | ||
Context appContext = createPackageContext(p.packageName, | ||
CONTEXT_INCLUDE_CODE | CONTEXT_IGNORE_SECURITY); | ||
ClassLoader classLoader = appContext.getClassLoader(); | ||
Object result = classLoader.loadClass("some.package.SomeClass") | ||
.getMethod("someMethod") | ||
.invoke(null); | ||
} | ||
} catch (Exception e) { | ||
Log.e("Class loading failed", e.toString()); | ||
} | ||
} | ||
} | ||
} |
31 changes: 31 additions & 0 deletions
31
java/ql/test/experimental/query-tests/security/CWE-470/GoodClassLoader.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package poc.sample.classloader; | ||
|
||
import android.app.Application; | ||
import android.content.pm.PackageInfo; | ||
import android.content.Context; | ||
import android.content.pm.PackageManager; | ||
import android.util.Log; | ||
|
||
public class GoodClassLoader extends Application { | ||
@Override | ||
public void onCreate() { | ||
super.onCreate(); | ||
PackageManager pm = getPackageManager(); | ||
for (PackageInfo p : pm.getInstalledPackages(0)) { | ||
try { | ||
if (p.packageName.startsWith("some.package.") && | ||
(pm.checkSignatures(p.packageName, getApplicationContext().getPackageName()) == PackageManager.SIGNATURE_MATCH) | ||
) { | ||
Context appContext = createPackageContext(p.packageName, | ||
CONTEXT_INCLUDE_CODE | CONTEXT_IGNORE_SECURITY); | ||
ClassLoader classLoader = appContext.getClassLoader(); | ||
Object result = classLoader.loadClass("some.package.SomeClass") | ||
.getMethod("someMethod") | ||
.invoke(null); | ||
} | ||
} catch (Exception e) { | ||
Log.e("Class loading failed", e.toString()); | ||
} | ||
} | ||
} | ||
} |
12 changes: 12 additions & 0 deletions
12
java/ql/test/experimental/query-tests/security/CWE-470/LoadClassNoSignatureCheck.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
edges | ||
| BadClassLoader.java:15:42:16:75 | createPackageContext(...) : Context | BadClassLoader.java:17:47:17:56 | appContext : Context | | ||
| BadClassLoader.java:17:47:17:56 | appContext : Context | BadClassLoader.java:17:47:17:73 | getClassLoader(...) : ClassLoader | | ||
| BadClassLoader.java:17:47:17:73 | getClassLoader(...) : ClassLoader | BadClassLoader.java:18:37:18:47 | classLoader | | ||
nodes | ||
| BadClassLoader.java:15:42:16:75 | createPackageContext(...) : Context | semmle.label | createPackageContext(...) : Context | | ||
| BadClassLoader.java:17:47:17:56 | appContext : Context | semmle.label | appContext : Context | | ||
| BadClassLoader.java:17:47:17:73 | getClassLoader(...) : ClassLoader | semmle.label | getClassLoader(...) : ClassLoader | | ||
| BadClassLoader.java:18:37:18:47 | classLoader | semmle.label | classLoader | | ||
subpaths | ||
#select | ||
| BadClassLoader.java:18:37:18:47 | classLoader | BadClassLoader.java:15:42:16:75 | createPackageContext(...) : Context | BadClassLoader.java:18:37:18:47 | classLoader | Class loaded from a $@ without signature check | BadClassLoader.java:15:42:16:75 | createPackageContext(...) | third party library | |
1 change: 1 addition & 0 deletions
1
java/ql/test/experimental/query-tests/security/CWE-470/LoadClassNoSignatureCheck.qlref
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
experimental/Security/CWE/CWE-470/LoadClassNoSignatureCheck.ql |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/springframework-5.3.8/ | ||
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/springframework-5.3.8/:${testdir}/../../../../stubs/google-android-9.0.0 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.