Skip to content

Commit 75a2b94

Browse files
joefarebrotherjoefarebrother
authored
Merge pull request github#15481 from joefarebrother/android-local-auth
Java: Add query for insecure local authentication
2 parents db2eb20 + d3fea40 commit 75a2b94

21 files changed

+624
-18
lines changed

‎java/ql/lib/semmle/code/java/security/AndroidLocalAuthQuery.qll

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/** Definitions for the insecure local authentication query. */
2+
3+
import java
4+
5+
/** A base class that is used as a callback for biometric authentication. */
6+
private class AuthenticationCallbackClass extends Class {
7+
AuthenticationCallbackClass() {
8+
this.hasQualifiedName("android.hardware.fingerprint",
9+
"FingerprintManager$AuthenticationCallback")
10+
or
11+
this.hasQualifiedName("android.hardware.biometrics", "BiometricPrompt$AuthenticationCallback")
12+
or
13+
this.hasQualifiedName("androidx.biometric", "BiometricPrompt$AuthenticationCallback")
14+
}
15+
}
16+
17+
/** An implementation of the `onAuthenticationSucceeded` method for an authentication callback. */
18+
class AuthenticationSuccessCallback extends Method {
19+
AuthenticationSuccessCallback() {
20+
this.getDeclaringType().getASupertype+() instanceof AuthenticationCallbackClass and
21+
this.hasName("onAuthenticationSucceeded")
22+
}
23+
24+
/** Gets the parameter containing the `authenticationResult`. */
25+
Parameter getResultParameter() { result = this.getParameter(0) }
26+
27+
/** Gets a use of the result parameter that's used in a `super` call to the base `AuthenticationCallback` class. */
28+
private VarAccess getASuperResultUse() {
29+
exists(SuperMethodCall sup |
30+
sup.getEnclosingCallable() = this and
31+
result = sup.getArgument(0) and
32+
result = this.getResultParameter().getAnAccess() and
33+
this.getDeclaringType().getASupertype() instanceof AuthenticationCallbackClass
34+
)
35+
}
36+
37+
/** Gets a use of the result parameter, other than one used in a `super` call. */
38+
VarAccess getAResultUse() {
39+
result = this.getResultParameter().getAnAccess() and
40+
not result = this.getASuperResultUse()
41+
}
42+
}

‎java/ql/src/Security/CWE/CWE-287/AndroidInsecureLocalAuthentication.qhelp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Biometric local authentication such as fingerprint recognition can be used to protect sensitive data or actions within an application.
9+
However, if this authentication does not use a <code>KeyStore</code>-backed key, it can be bypassed by a privileged malicious application, or by an attacker with physical access using application hooking tools such as Frida.
10+
</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>
15+
Generate a secure key in the Android <code>KeyStore</code>. Ensure that the <code>onAuthenticationSuccess</code> callback for a biometric prompt uses it
16+
in a way that is required for the sensitive parts of the application to function, such as by using it to decrypt sensitive data or credentials.
17+
</p>
18+
</recommendation>
19+
20+
<example>
21+
<p>In the following (bad) case, no <code>CryptoObject</code> is required for the biometric prompt to grant access, so it can be bypassed.</p>
22+
<sample src="AndroidInsecureLocalAuthenticationBad.java" />
23+
<p>In the following (good) case, a secret key is generated in the Android <code>KeyStore</code>. The application requires this secret key for access, using it to decrypt data.</p>
24+
<sample src="AndroidInsecureLocalAuthenticationGood.java" />
25+
</example>
26+
27+
<references>
28+
<li>
29+
OWASP Mobile Application Security: <a href="https://mas.owasp.org/MASTG/Android/0x05f-Testing-Local-Authentication/">Android Local Authentication</a>
30+
</li>
31+
<li>
32+
OWASP Mobile Application Security: <a href="https://mas.owasp.org/MASTG/tests/android/MASVS-AUTH/MASTG-TEST-0018/">Testing Biometric Authentication</a>
33+
</li>
34+
<li>
35+
WithSecure: <a href="https://labs.withsecure.com/publications/how-secure-is-your-android-keystore-authentication">How Secure is your Android Keystore Authentication?</a>
36+
</li>
37+
<li>
38+
Android Developers: <a href="https://developer.android.com/training/sign-in/biometric-auth">Biometric Authentication</a>
39+
</li>
40+
41+
</references>
42+
</qhelp>

‎java/ql/src/Security/CWE/CWE-287/AndroidInsecureLocalAuthentication.ql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* @name Insecure local authentication
3+
* @description Local authentication that does not make use of a `CryptoObject` can be bypassed.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @security-severity 4.4
7+
* @precision high
8+
* @id java/android/insecure-local-authentication
9+
* @tags security
10+
* external/cwe/cwe-287
11+
*/
12+
13+
import java
14+
import semmle.code.java.security.AndroidLocalAuthQuery
15+
16+
from AuthenticationSuccessCallback c
17+
where not exists(c.getAResultUse())
18+
select c, "This authentication callback does not use its result for a cryptographic operation."

‎java/ql/src/Security/CWE/CWE-287/AndroidInsecureLocalAuthenticationBad.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
biometricPrompt.authenticate(
2+
cancellationSignal,
3+
executor,
4+
new BiometricPrompt.AuthenticationCallback {
5+
@Override
6+
// BAD: This authentication callback does not make use of a `CryptoObject` from the `result`.
7+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
8+
grantAccess()
9+
}
10+
}
11+
)

‎java/ql/src/Security/CWE/CWE-287/AndroidInsecureLocalAuthenticationGood.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
private void generateSecretKey() {
2+
KeyGenParameterSpec keyGenParameterSpec = new KeyGenParameterSpec.Builder(
3+
"MySecretKey",
4+
KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT)
5+
.setBlockModes(KeyProperties.BLOCK_MODE_CBC)
6+
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
7+
.setUserAuthenticationRequired(true)
8+
.setInvalidatedByBiometricEnrollment(true)
9+
.build();
10+
KeyGenerator keyGenerator = KeyGenerator.getInstance(
11+
KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore");
12+
keyGenerator.init(keyGenParameterSpec);
13+
keyGenerator.generateKey();
14+
}
15+
16+
17+
private SecretKey getSecretKey() {
18+
KeyStore keyStore = KeyStore.getInstance("AndroidKeyStore");
19+
keyStore.load(null);
20+
return ((SecretKey)keyStore.getKey("MySecretKey", null));
21+
}
22+
23+
private Cipher getCipher() {
24+
return Cipher.getInstance(KeyProperties.KEY_ALGORITHM_AES + "/"
25+
+ KeyProperties.BLOCK_MODE_CBC + "/"
26+
+ KeyProperties.ENCRYPTION_PADDING_PKCS7);
27+
}
28+
29+
public prompt(byte[] encryptedData) {
30+
Cipher cipher = getCipher();
31+
SecretKey secretKey = getSecretKey();
32+
cipher.init(Cipher.DECRYPT_MODE, secretKey);
33+
34+
biometricPrompt.authenticate(
35+
new BiometricPrompt.CryptoObject(cipher),
36+
cancellationSignal,
37+
executor,
38+
new BiometricPrompt.AuthenticationCallback() {
39+
@Override
40+
// GOOD: This authentication callback uses the result to decrypt some data.
41+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
42+
Cipher cipher = result.getCryptoObject().getCipher();
43+
byte[] decryptedData = cipher.doFinal(encryptedData);
44+
grantAccessWithData(decryptedData);
45+
}
46+
}
47+
);
48+
}

‎java/ql/src/change-notes/2024-02-02-android-insecure-local-auth.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
2+
---
3+
category: newQuery
4+
---
5+
* Added a new query `java/android/insecure-local-authentication` for finding uses of biometric authentication APIs that do not make use of a `KeyStore`-backed key and thus may be bypassed.

‎java/ql/test/query-tests/security/CWE-287/InsecureLocalAuth.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
testFailures
2+
failures

‎java/ql/test/query-tests/security/CWE-287/InsecureLocalAuth.ql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import java
2+
import TestUtilities.InlineExpectationsTest
3+
import semmle.code.java.dataflow.DataFlow
4+
import semmle.code.java.security.AndroidLocalAuthQuery
5+
6+
module InsecureAuthTest implements TestSig {
7+
string getARelevantTag() { result = "insecure-auth" }
8+
9+
predicate hasActualResult(Location location, string element, string tag, string value) {
10+
tag = "insecure-auth" and
11+
exists(AuthenticationSuccessCallback cb | not exists(cb.getAResultUse()) |
12+
cb.getLocation() = location and
13+
element = cb.toString() and
14+
value = ""
15+
)
16+
}
17+
}
18+
19+
import MakeTest<InsecureAuthTest>

‎java/ql/test/query-tests/security/CWE-287/Test.java

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import android.hardware.biometrics.BiometricPrompt;
2+
import android.hardware.fingerprint.FingerprintManager;
3+
4+
class TestA {
5+
public static void useKey(BiometricPrompt.CryptoObject key) {}
6+
7+
8+
// GOOD: result is used
9+
class Test1 extends BiometricPrompt.AuthenticationCallback {
10+
@Override
11+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
12+
TestA.useKey(result.getCryptoObject());
13+
}
14+
}
15+
16+
// BAD: result is not used
17+
class Test2 extends BiometricPrompt.AuthenticationCallback {
18+
@Override
19+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) { // $insecure-auth
20+
21+
}
22+
}
23+
24+
// BAD: result is only used in a super call
25+
class Test3 extends BiometricPrompt.AuthenticationCallback {
26+
@Override
27+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) { // $insecure-auth
28+
super.onAuthenticationSucceeded(result);
29+
}
30+
}
31+
32+
// GOOD: result is used
33+
class Test4 extends BiometricPrompt.AuthenticationCallback {
34+
@Override
35+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
36+
super.onAuthenticationSucceeded(result);
37+
TestA.useKey(result.getCryptoObject());
38+
}
39+
}
40+
41+
// GOOD: result is used in a super call to a class other than the base class
42+
class Test5 extends Test1 {
43+
@Override
44+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
45+
super.onAuthenticationSucceeded(result);
46+
}
47+
}
48+
}
49+
50+
class TestB {
51+
public static void useKey(FingerprintManager.CryptoObject key) {}
52+
53+
54+
// GOOD: result is used
55+
class Test1 extends FingerprintManager.AuthenticationCallback {
56+
@Override
57+
public void onAuthenticationSucceeded(FingerprintManager.AuthenticationResult result) {
58+
TestB.useKey(result.getCryptoObject());
59+
}
60+
}
61+
62+
// BAD: result is not used
63+
class Test2 extends FingerprintManager.AuthenticationCallback {
64+
@Override
65+
public void onAuthenticationSucceeded(FingerprintManager.AuthenticationResult result) { // $insecure-auth
66+
67+
}
68+
}
69+
70+
// BAD: result is only used in a super call
71+
class Test3 extends FingerprintManager.AuthenticationCallback {
72+
@Override
73+
public void onAuthenticationSucceeded(FingerprintManager.AuthenticationResult result) { // $insecure-auth
74+
super.onAuthenticationSucceeded(result);
75+
}
76+
}
77+
78+
// GOOD: result is used
79+
class Test4 extends FingerprintManager.AuthenticationCallback {
80+
@Override
81+
public void onAuthenticationSucceeded(FingerprintManager.AuthenticationResult result) {
82+
super.onAuthenticationSucceeded(result);
83+
TestB.useKey(result.getCryptoObject());
84+
}
85+
}
86+
87+
// GOOD: result is used in a super call to a class other than the base class
88+
class Test5 extends Test1 {
89+
@Override
90+
public void onAuthenticationSucceeded(FingerprintManager.AuthenticationResult result) {
91+
super.onAuthenticationSucceeded(result);
92+
}
93+
}
94+
}

‎java/ql/test/query-tests/security/CWE-287/Test2.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import androidx.biometric.BiometricPrompt;
2+
3+
class TestC {
4+
public static void useKey(BiometricPrompt.CryptoObject key) {}
5+
6+
7+
// GOOD: result is used
8+
class Test1 extends BiometricPrompt.AuthenticationCallback {
9+
@Override
10+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
11+
TestC.useKey(result.getCryptoObject());
12+
}
13+
}
14+
15+
// BAD: result is not used
16+
class Test2 extends BiometricPrompt.AuthenticationCallback {
17+
@Override
18+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) { // $insecure-auth
19+
20+
}
21+
}
22+
23+
// BAD: result is only used in a super call
24+
class Test3 extends BiometricPrompt.AuthenticationCallback {
25+
@Override
26+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) { // $insecure-auth
27+
super.onAuthenticationSucceeded(result);
28+
}
29+
}
30+
31+
// GOOD: result is used
32+
class Test4 extends BiometricPrompt.AuthenticationCallback {
33+
@Override
34+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
35+
super.onAuthenticationSucceeded(result);
36+
TestC.useKey(result.getCryptoObject());
37+
}
38+
}
39+
40+
// GOOD: result is used in a super call to a class other than the base class
41+
class Test5 extends Test1 {
42+
@Override
43+
public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
44+
super.onAuthenticationSucceeded(result);
45+
}
46+
}
47+
}

0 commit comments

Comments
 (0)