Merge branch 'main' into redsun82/bzlmod

Author: redsun82

java/ql/lib/change-notes/2024-02-09-url-redirect-sanitizer.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* An extension point for sanitizers of the query `java/unvalidated-url-redirection` has been added.

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

@@ -0,0 +1,42 @@
+/** Definitions for the insecure local authentication query. */
+
+import java
+
+/** A base class that is used as a callback for biometric authentication. */
+private class AuthenticationCallbackClass extends Class {
+  AuthenticationCallbackClass() {
+    this.hasQualifiedName("android.hardware.fingerprint",
+      "FingerprintManager$AuthenticationCallback")
+    or
+    this.hasQualifiedName("android.hardware.biometrics", "BiometricPrompt$AuthenticationCallback")
+    or
+    this.hasQualifiedName("androidx.biometric", "BiometricPrompt$AuthenticationCallback")
+  }
+}
+
+/** An implementation of the `onAuthenticationSucceeded` method for an authentication callback. */
+class AuthenticationSuccessCallback extends Method {
+  AuthenticationSuccessCallback() {
+    this.getDeclaringType().getASupertype+() instanceof AuthenticationCallbackClass and
+    this.hasName("onAuthenticationSucceeded")
+  }
+
+  /** Gets the parameter containing the `authenticationResult`. */
+  Parameter getResultParameter() { result = this.getParameter(0) }
+
+  /** Gets a use of the result parameter that's used in a `super` call to the base `AuthenticationCallback` class. */
+  private VarAccess getASuperResultUse() {
+    exists(SuperMethodCall sup |
+      sup.getEnclosingCallable() = this and
+      result = sup.getArgument(0) and
+      result = this.getResultParameter().getAnAccess() and
+      this.getDeclaringType().getASupertype() instanceof AuthenticationCallbackClass
+    )
+  }
+
+  /** Gets a use of the result parameter, other than one used in a `super` call. */
+  VarAccess getAResultUse() {
+    result = this.getResultParameter().getAnAccess() and
+    not result = this.getASuperResultUse()
+  }
+}

java/ql/lib/semmle/code/java/security/UrlRedirect.qll

@@ -6,10 +6,14 @@ private import semmle.code.java.dataflow.ExternalFlow
 import semmle.code.java.frameworks.Servlets
 import semmle.code.java.frameworks.ApacheHttp
 private import semmle.code.java.frameworks.JaxWS
+private import semmle.code.java.security.RequestForgery
 
 /** A URL redirection sink. */
 abstract class UrlRedirectSink extends DataFlow::Node { }
 
+/** A URL redirection sanitizer. */
+abstract class UrlRedirectSanitizer extends DataFlow::Node { }
+
 /** A default sink represeting methods susceptible to URL redirection attacks. */
 private class DefaultUrlRedirectSink extends UrlRedirectSink {
   DefaultUrlRedirectSink() { sinkNode(this, "url-redirection") }
@@ -42,3 +46,6 @@ private class ApacheUrlRedirectSink extends UrlRedirectSink {
     )
   }
 }
+
+private class DefaultUrlRedirectSanitizer extends UrlRedirectSanitizer instanceof RequestForgerySanitizer
+{ }

java/ql/lib/semmle/code/java/security/UrlRedirectQuery.qll

@@ -11,6 +11,8 @@ module UrlRedirectConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }
 
   predicate isSink(DataFlow::Node sink) { sink instanceof UrlRedirectSink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof UrlRedirectSanitizer }
 }
 
 /**

java/ql/src/Security/CWE/CWE-078/ExecRelative.ql

@@ -4,7 +4,7 @@
  *              malicious changes in the PATH environment variable.
  * @kind problem
  * @problem.severity warning
- * @security-severity 9.8
+ * @security-severity 5.4
  * @precision medium
  * @id java/relative-path-command
  * @tags security

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

@@ -0,0 +1,42 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Biometric local authentication such as fingerprint recognition can be used to protect sensitive data or actions within an application. 
+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.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Generate a secure key in the Android <code>KeyStore</code>. Ensure that the <code>onAuthenticationSuccess</code> callback for a biometric prompt uses it 
+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. 
+</p>
+</recommendation>
+
+<example>
+<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>
+<sample src="AndroidInsecureLocalAuthenticationBad.java" />
+<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>
+<sample src="AndroidInsecureLocalAuthenticationGood.java" />
+</example>
+
+<references>
+<li>
+OWASP Mobile Application Security: <a href="https://mas.owasp.org/MASTG/Android/0x05f-Testing-Local-Authentication/">Android Local Authentication</a>
+</li>
+<li>
+OWASP Mobile Application Security: <a href="https://mas.owasp.org/MASTG/tests/android/MASVS-AUTH/MASTG-TEST-0018/">Testing Biometric Authentication</a>
+</li>
+<li>
+WithSecure: <a href="https://labs.withsecure.com/publications/how-secure-is-your-android-keystore-authentication">How Secure is your Android Keystore Authentication?</a>
+</li>
+<li>
+Android Developers: <a href="https://developer.android.com/training/sign-in/biometric-auth">Biometric Authentication</a>
+</li>
+
+</references>
+</qhelp>

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

@@ -0,0 +1,18 @@
+/**
+ * @name Insecure local authentication
+ * @description Local authentication that does not make use of a `CryptoObject` can be bypassed.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 4.4
+ * @precision high
+ * @id java/android/insecure-local-authentication
+ * @tags security
+ *       external/cwe/cwe-287
+ */
+
+import java
+import semmle.code.java.security.AndroidLocalAuthQuery
+
+from AuthenticationSuccessCallback c
+where not exists(c.getAResultUse())
+select c, "This authentication callback does not use its result for a cryptographic operation."

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

@@ -0,0 +1,11 @@
+biometricPrompt.authenticate(
+    cancellationSignal,
+    executor,
+    new BiometricPrompt.AuthenticationCallback {
+        @Override
+        // BAD: This authentication callback does not make use of a `CryptoObject` from the `result`.
+        public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
+            grantAccess()
+        }
+    }
+)

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

@@ -0,0 +1,48 @@
+private void generateSecretKey() {
+    KeyGenParameterSpec keyGenParameterSpec = new KeyGenParameterSpec.Builder(
+        "MySecretKey",
+        KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT)
+        .setBlockModes(KeyProperties.BLOCK_MODE_CBC)
+        .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
+        .setUserAuthenticationRequired(true)
+        .setInvalidatedByBiometricEnrollment(true)
+        .build();
+    KeyGenerator keyGenerator = KeyGenerator.getInstance(
+            KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore");
+    keyGenerator.init(keyGenParameterSpec);
+    keyGenerator.generateKey();
+}
+
+
+private SecretKey getSecretKey() {
+    KeyStore keyStore = KeyStore.getInstance("AndroidKeyStore");
+    keyStore.load(null);
+    return ((SecretKey)keyStore.getKey("MySecretKey", null));
+}
+
+private Cipher getCipher() {
+    return Cipher.getInstance(KeyProperties.KEY_ALGORITHM_AES + "/"
+            + KeyProperties.BLOCK_MODE_CBC + "/"
+            + KeyProperties.ENCRYPTION_PADDING_PKCS7);
+}
+
+public prompt(byte[] encryptedData) {
+    Cipher cipher = getCipher();
+    SecretKey secretKey = getSecretKey();
+    cipher.init(Cipher.DECRYPT_MODE, secretKey);
+
+    biometricPrompt.authenticate(
+        new BiometricPrompt.CryptoObject(cipher),
+        cancellationSignal,
+        executor,
+        new BiometricPrompt.AuthenticationCallback() {
+            @Override
+            // GOOD: This authentication callback uses the result to decrypt some data.
+            public void onAuthenticationSucceeded(BiometricPrompt.AuthenticationResult result) {
+                Cipher cipher = result.getCryptoObject().getCipher();
+                byte[] decryptedData = cipher.doFinal(encryptedData);
+                grantAccessWithData(decryptedData);
+            }
+        }
+    );
+}

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

@@ -0,0 +1,5 @@
+
+---
+category: newQuery
+---
+* 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.