SONARKT-164 S6293: Fix false-negatives for android.hardware.biometrics.BiometricPrompt (#136)

Author: margarita-nedzelska-sonarsource

kotlin-checks-test-sources/src/main/java/android/hardware/biometrics/BiometricPrompt.java

@@ -0,0 +1,23 @@
+package android.hardware.biometrics;
+
+import android.os.CancellationSignal;
+
+import java.util.concurrent.Executor;
+
+public class BiometricPrompt {
+  public static class AuthenticationCallback {
+
+  }
+
+  public static final class CryptoObject {
+
+  }
+
+  public void authenticate(CancellationSignal cancel, Executor executor, AuthenticationCallback callback) {
+
+  }
+
+  public void authenticate(CryptoObject crypto, CancellationSignal cancel, Executor executor, AuthenticationCallback callback) {
+
+  }
+}

kotlin-checks-test-sources/src/main/java/android/os/Bundle.java

@@ -0,0 +1,4 @@
+package android.os;
+
+public class Bundle {
+}

kotlin-checks-test-sources/src/main/java/android/os/CancellationSignal.java

@@ -0,0 +1,4 @@
+package android.os;
+
+public class CancellationSignal {
+}

kotlin-checks-test-sources/src/main/java/androidx/biometric/BiometricPrompt.java

@@ -0,0 +1,17 @@
+package androidx.biometric;
+
+public class BiometricPrompt {
+  public static class CryptoObject {
+
+  }
+
+  public static class PromptInfo {
+
+  }
+
+  public void authenticate(PromptInfo info, CryptoObject crypto) {
+  }
+
+  public void authenticate(PromptInfo info) {
+  }
+}

kotlin-checks-test-sources/src/main/kotlin/android/hardware/biometrics/BiometricPrompt.kt

@@ -1,20 +1,21 @@
 package checks
 
+import android.os.CancellationSignal
 import android.hardware.biometrics.BiometricPrompt as BiometricPromptAndroid
 import androidx.biometric.BiometricPrompt as BiometricPromptAndroidX
 
-val promptInfo = ""
-val cipher = ""
+val promptInfo = androidx.biometric.BiometricPrompt.PromptInfo()
 
 fun android() {
     val biometricPrompt = BiometricPromptAndroid()
 
-    biometricPrompt.authenticate(promptInfo) // Noncompliant {{Make sure performing a biometric authentication without a "CryptoObject" is safe here.}}
+    biometricPrompt.authenticate(CancellationSignal(), {  }, BiometricPromptAndroid.AuthenticationCallback()) // Noncompliant {{Make sure performing a biometric authentication without a "CryptoObject" is safe here.}}
+    
     // Noncompliant@+1
-    biometricPrompt.authenticate(promptInfo, null)
-//                  ^^^^^^^^^^^^>            ^^^^
+    biometricPrompt.authenticate     (null, CancellationSignal(), {  }, BiometricPromptAndroid.AuthenticationCallback())
+//                  ^^^^^^^^^^^^>     ^^^^
 
-    biometricPrompt.authenticate(promptInfo, BiometricPromptAndroid.CryptoObject(cipher)) // Compliant
+    biometricPrompt.authenticate(BiometricPromptAndroid.CryptoObject(), CancellationSignal(), {  }, BiometricPromptAndroid.AuthenticationCallback()) // Compliant
 }
 
 fun androidx() {
@@ -25,5 +26,5 @@ fun androidx() {
     biometricPrompt.authenticate(promptInfo, null)
 //                  ^^^^^^^^^^^^>            ^^^^
 
-    biometricPrompt.authenticate(promptInfo, BiometricPromptAndroidX.CryptoObject(cipher))  // Compliant
+    biometricPrompt.authenticate(promptInfo, BiometricPromptAndroidX.CryptoObject())  // Compliant
 }

kotlin-checks-test-sources/src/main/kotlin/android/os/Bundle.kt

@@ -22,48 +22,48 @@ package org.sonarsource.kotlin.checks
 import org.jetbrains.kotlin.psi.KtCallExpression
 import org.jetbrains.kotlin.psi.psiUtil.getCallNameExpression
 import org.jetbrains.kotlin.psi.psiUtil.isNull
-import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
+import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
 import org.sonar.check.Rule
-import org.sonarsource.kotlin.api.AbstractCheck
+import org.sonarsource.kotlin.api.CallAbstractCheck
 import org.sonarsource.kotlin.api.FunMatcher
+import org.sonarsource.kotlin.api.FunMatcherImpl
 import org.sonarsource.kotlin.api.SecondaryLocation
-import org.sonarsource.kotlin.api.matches
 import org.sonarsource.kotlin.converter.KotlinTextRanges.textRange
 import org.sonarsource.kotlin.plugin.KotlinFileContext
 
-private val AUTHENTICATE_FUN_MATCHERS = listOf(
-    FunMatcher(qualifier = "android.hardware.biometrics.BiometricPrompt", name = "authenticate"),
-    FunMatcher(qualifier = "androidx.biometric.BiometricPrompt", name = "authenticate")
-)
+private val ANDROID_HARDWARE_AUTH = FunMatcher(qualifier = "android.hardware.biometrics.BiometricPrompt", name = "authenticate")
+private val ANDROIDX_AUTH = FunMatcher(qualifier = "androidx.biometric.BiometricPrompt", name = "authenticate")
 
 private const val MESSAGE = """Make sure performing a biometric authentication without a "CryptoObject" is safe here."""
 
 @Rule(key = "S6293")
-class BiometricAuthWithoutCryptoCheck : AbstractCheck() {
+class BiometricAuthWithoutCryptoCheck : CallAbstractCheck() {
+    override val functionsToVisit = setOf(ANDROID_HARDWARE_AUTH, ANDROIDX_AUTH)
 
-    override fun visitCallExpression(callExpression: KtCallExpression, ctx: KotlinFileContext) {
-        if (functionCallMatches(callExpression, ctx)) {
-            if (callExpression.valueArguments.size < 2) {
-
-                // No second argument -> automatically insecure.
-                ctx.reportIssue(callExpression, MESSAGE)
-
-            } else {
+    override fun visitFunctionCall(
+        callExpression: KtCallExpression,
+        resolvedCall: ResolvedCall<*>,
+        matchedFun: FunMatcherImpl,
+        kotlinFileContext: KotlinFileContext,
+    ) {
+        when(matchedFun) {
+            ANDROID_HARDWARE_AUTH -> checkCall(callExpression, kotlinFileContext, 4, 0)
+            ANDROIDX_AUTH -> checkCall(callExpression, kotlinFileContext, 2, 1)
+        }
+    }
 
-                // Second argument is null? Also insecure.
-                callExpression.valueArguments[1].getArgumentExpression()?.let { relevantArg ->
-                    if (relevantArg.isNull()) {
-                        val secondaryExpression = callExpression.getCallNameExpression() ?: callExpression
-                        ctx.reportIssue(relevantArg, MESSAGE, listOf(SecondaryLocation(ctx.textRange(secondaryExpression))))
-                    }
+    private fun checkCall(callExpression: KtCallExpression, ctx: KotlinFileContext, numberOfSafeArgs: Int, argumentIndex: Int) {
+        if (callExpression.valueArguments.size < numberOfSafeArgs) {
+            // No CryptoObject -> automatically insecure.
+            ctx.reportIssue(callExpression, MESSAGE)
+        } else {
+            // CryptoObject is null -> also insecure.
+            callExpression.valueArguments[argumentIndex].getArgumentExpression()?.let { relevantArg ->
+                if (relevantArg.isNull()) {
+                    val secondaryExpression = callExpression.getCallNameExpression() ?: callExpression
+                    ctx.reportIssue(relevantArg, MESSAGE, listOf(SecondaryLocation(ctx.textRange(secondaryExpression))))
                 }
-
             }
         }
     }
-
-    private fun functionCallMatches(callExpression: KtCallExpression, ctx: KotlinFileContext) =
-        callExpression.getResolvedCall(ctx.bindingContext)?.let { resolvedCall ->
-            AUTHENTICATE_FUN_MATCHERS.any { resolvedCall matches it }
-        } ?: false
 }