Add null check before accessing mCurrentChallengeOwner

Fixes: 169151846
Test: atest com.android.server.biometrics

Change-Id: Iec95b5f9171bd722ff13c2c729113548fb2e508a

2 files changed, 92 insertions, 7 deletions

diff --git a/services/core/java/com/android/server/biometrics/sensors/face/Face10.java b/services/core/java/com/android/server/biometrics/sensors/face/Face10.java
index bff0c3c686ef..49a4b7c4deb3 100644
--- a/services/core/java/com/android/server/biometrics/sensors/face/Face10.java
+++ b/services/core/java/com/android/server/biometrics/sensors/face/Face10.java
@@ -48,6 +48,7 @@ import android.provider.Settings;
 import android.util.Slog;
 
 import com.android.internal.R;
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.FrameworkStatsLog;
 import com.android.server.biometrics.Utils;
 import com.android.server.biometrics.sensors.AcquisitionClient;
@@ -278,14 +279,13 @@ class Face10 implements IHwBinder.DeathRecipient {
         }
     };
 
+    @VisibleForTesting
     Face10(@NonNull Context context, int sensorId,
             @BiometricManager.Authenticators.Types int strength,
-            @NonNull LockoutResetDispatcher lockoutResetDispatcher) {
-        final boolean supportsSelfIllumination = context.getResources()
-                .getBoolean(R.bool.config_faceAuthSupportsSelfIllumination);
-        final int maxTemplatesAllowed = context.getResources()
-                .getInteger(R.integer.config_faceMaxTemplatesPerUser);
-        mFaceSensorProperties = new FaceSensorPropertiesInternal(sensorId, strength,
+            @NonNull LockoutResetDispatcher lockoutResetDispatcher,
+            boolean supportsSelfIllumination, int maxTemplatesAllowed) {
+        mFaceSensorProperties = new FaceSensorPropertiesInternal(sensorId,
+                Utils.authenticatorStrengthToPropertyStrength(strength),
                 maxTemplatesAllowed, false /* supportsFaceDetect */, supportsSelfIllumination);
         mContext = context;
         mSensorId = sensorId;
@@ -305,6 +305,14 @@ class Face10 implements IHwBinder.DeathRecipient {
         }
     }
 
+    Face10(@NonNull Context context, int sensorId,
+            @BiometricManager.Authenticators.Types int strength,
+            @NonNull LockoutResetDispatcher lockoutResetDispatcher) {
+        this(context, sensorId, strength, lockoutResetDispatcher,
+                context.getResources().getBoolean(R.bool.config_faceAuthSupportsSelfIllumination),
+                context.getResources().getInteger(R.integer.config_faceMaxTemplatesPerUser));
+    }
+
     @Override
     public void serviceDied(long cookie) {
         Slog.e(TAG, "HAL died");
@@ -547,7 +555,8 @@ class Face10 implements IHwBinder.DeathRecipient {
 
     void scheduleRevokeChallenge(@NonNull IBinder token, @NonNull String owner) {
         mHandler.post(() -> {
-            if (!mCurrentChallengeOwner.getOwnerString().contentEquals(owner)) {
+            if (mCurrentChallengeOwner != null &&
+                    !mCurrentChallengeOwner.getOwnerString().contentEquals(owner)) {
                 Slog.e(TAG, "scheduleRevokeChallenge, package: " + owner
                         + " attempting to revoke challenge owned by: "
                         + mCurrentChallengeOwner.getOwnerString());
@@ -566,6 +575,13 @@ class Face10 implements IHwBinder.DeathRecipient {
                         return;
                     }
 
+                    if (mCurrentChallengeOwner == null) {
+                        // Can happen if revoke is incorrectly called, for example without a
+                        // preceding generateChallenge
+                        Slog.w(TAG, "Current challenge owner is null");
+                        return;
+                    }
+
                     final FaceGenerateChallengeClient previousChallengeOwner =
                             mCurrentChallengeOwner.getInterruptedClient();
                     mCurrentChallengeOwner = null;
diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/Face10Test.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/Face10Test.java
new file mode 100644
index 000000000000..7a0d8946d179
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/Face10Test.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.biometrics.sensors.face;
+
+import android.content.Context;
+import android.hardware.biometrics.BiometricManager;
+import android.os.Binder;
+import android.os.IBinder;
+import android.platform.test.annotations.Presubmit;
+
+import androidx.test.InstrumentationRegistry;
+import androidx.test.filters.SmallTest;
+
+import com.android.server.biometrics.sensors.LockoutResetDispatcher;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+@Presubmit
+@SmallTest
+public class Face10Test {
+
+    private static final String TAG = "Face10Test";
+    private static final int SENSOR_ID = 1;
+
+    @Mock
+    private Context mContext;
+
+    private LockoutResetDispatcher mLockoutResetDispatcher;
+    private Face10 mFace10;
+    private IBinder mBinder;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+
+        mLockoutResetDispatcher = new LockoutResetDispatcher(mContext);
+        mFace10 = new Face10(mContext, SENSOR_ID, BiometricManager.Authenticators.BIOMETRIC_STRONG,
+                mLockoutResetDispatcher, false /* supportsSelfIllumination */,
+                1 /* maxTemplatesAllowed */);
+        mBinder = new Binder();
+    }
+
+    @Test
+    public void scheduleRevokeChallenge_doesNotCrash() {
+        mFace10.scheduleRevokeChallenge(mBinder, TAG);
+        waitForIdle();
+    }
+
+    private static void waitForIdle() {
+        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
+    }
+}