From f0f6d42bd537bfbb036d459cc19d84ab6db6857e Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 11 Jan 2023 22:54:51 +0000
Subject: [PATCH] LockSettingsService: reject null keys in setString() et al.

Make the setBoolean(), setLong(), and setString() methods of
ILockSettings throw an exception if the given key is null, rather than
allowing it to be inserted into the locksettings database, where it
previously would cause a crash upon preloading.

This is a small cleanup only; we probably should go further and use an
allowlist of key names.  Still, the ACCESS_KEYGUARD_SECURE_STORAGE
permission is required to call these methods, so there doesn't appear to
be any security impact from the lax input validation.

Bug: 261860102
Test: com.android.server.locksettings
Change-Id: Ic8632068b92182425e78e2f28a1c4016b7af4a3b
---
 .../server/locksettings/LockSettingsService.java  |  3 +++
 .../locksettings/LockSettingsServiceTests.java    | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java
index 121b7c87223bf..31b8ef28f2464 100644
--- a/services/core/java/com/android/server/locksettings/LockSettingsService.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java
@@ -1156,18 +1156,21 @@ public class LockSettingsService extends ILockSettings.Stub {
     @Override
     public void setBoolean(String key, boolean value, int userId) {
         checkWritePermission();
+        Objects.requireNonNull(key);
         mStorage.setBoolean(key, value, userId);
     }
 
     @Override
     public void setLong(String key, long value, int userId) {
         checkWritePermission();
+        Objects.requireNonNull(key);
         mStorage.setLong(key, value, userId);
     }
 
     @Override
     public void setString(String key, String value, int userId) {
         checkWritePermission();
+        Objects.requireNonNull(key);
         mStorage.setString(key, value, userId);
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java
index 41e3a08be6c56..60a033fde4270 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java
@@ -423,6 +423,21 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {
         checkPasswordHistoryLength(userId, 3);
     }
 
+    @Test(expected=NullPointerException.class)
+    public void testSetBooleanRejectsNullKey() {
+        mService.setBoolean(null, false, 0);
+    }
+
+    @Test(expected=NullPointerException.class)
+    public void testSetLongRejectsNullKey() {
+        mService.setLong(null, 0, 0);
+    }
+
+    @Test(expected=NullPointerException.class)
+    public void testSetStringRejectsNullKey() {
+        mService.setString(null, "value", 0);
+    }
+
     private void checkPasswordHistoryLength(int userId, int expectedLen) {
         String history = mService.getString(LockPatternUtils.PASSWORD_HISTORY_KEY, "", userId);
         String[] hashes = TextUtils.split(history, LockPatternUtils.PASSWORD_HISTORY_DELIMITER);