mirror of
https://github.com/Divested-Mobile/DivestOS-Build.git
synced 2024-10-01 01:35:54 -04:00
250 lines
13 KiB
Diff
250 lines
13 KiB
Diff
|
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||
|
From: Songchun Fan <schfan@google.com>
|
||
|
Date: Tue, 11 Oct 2022 18:08:11 -0700
|
||
|
Subject: [PATCH] key size limit for mutating settings
|
||
|
|
||
|
Prior to targetSdk 22, apps could add random system settings keys which
|
||
|
opens an opportunity for OOM attacks. This CL adds a key size limit.
|
||
|
|
||
|
BUG: 239415997
|
||
|
Test: manual; will add cts test
|
||
|
Merged-In: Ic9e88c0cc3d7206c64ba5b5c7d15b50d1ffc9adc
|
||
|
Change-Id: Ic9e88c0cc3d7206c64ba5b5c7d15b50d1ffc9adc
|
||
|
(cherry picked from commit 783bcba343c480f6ccedaaff41ba7171a1082e0c)
|
||
|
(cherry picked from commit f1831c87122e56951c04e1f62f647ab156ca71e3)
|
||
|
Merged-In: Ic9e88c0cc3d7206c64ba5b5c7d15b50d1ffc9adc
|
||
|
---
|
||
|
.../providers/settings/SettingsState.java | 40 ++++---
|
||
|
.../providers/settings/SettingsStateTest.java | 102 +++++++++++++++++-
|
||
|
2 files changed, 126 insertions(+), 16 deletions(-)
|
||
|
|
||
|
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
|
||
|
index 0e94ea85919b..11b4c9eb1c1d 100644
|
||
|
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
|
||
|
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
|
||
|
@@ -46,6 +46,7 @@ import android.util.Xml;
|
||
|
import android.util.proto.ProtoOutputStream;
|
||
|
|
||
|
import com.android.internal.annotations.GuardedBy;
|
||
|
+import com.android.internal.annotations.VisibleForTesting;
|
||
|
import com.android.server.LocalServices;
|
||
|
|
||
|
import libcore.io.IoUtils;
|
||
|
@@ -306,8 +307,8 @@ final class SettingsState {
|
||
|
Setting newSetting = new Setting(name, oldSetting.getValue(), null,
|
||
|
oldSetting.getPackageName(), oldSetting.getTag(), false,
|
||
|
oldSetting.getId());
|
||
|
- int newSize = getNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue,
|
||
|
- newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue());
|
||
|
+ int newSize = getNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), 0,
|
||
|
+ oldValue, newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue());
|
||
|
checkNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize);
|
||
|
mSettings.put(name, newSetting);
|
||
|
updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize);
|
||
|
@@ -327,8 +328,9 @@ final class SettingsState {
|
||
|
String oldDefaultValue = (oldState != null) ? oldState.defaultValue : null;
|
||
|
String newDefaultValue = makeDefault ? value : oldDefaultValue;
|
||
|
|
||
|
- int newSize = getNewMemoryUsagePerPackageLocked(packageName, oldValue, value,
|
||
|
- oldDefaultValue, newDefaultValue);
|
||
|
+ int newSize = getNewMemoryUsagePerPackageLocked(packageName,
|
||
|
+ oldValue == null ? name.length() : 0 /* deltaKeySize */,
|
||
|
+ oldValue, value, oldDefaultValue, newDefaultValue);
|
||
|
checkNewMemoryUsagePerPackageLocked(packageName, newSize);
|
||
|
|
||
|
Setting newState;
|
||
|
@@ -365,8 +367,12 @@ final class SettingsState {
|
||
|
}
|
||
|
|
||
|
Setting oldState = mSettings.remove(name);
|
||
|
- int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName, oldState.value,
|
||
|
- null, oldState.defaultValue, null);
|
||
|
+ if (oldState == null) {
|
||
|
+ return false;
|
||
|
+ }
|
||
|
+ int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName,
|
||
|
+ -name.length() /* deltaKeySize */,
|
||
|
+ oldState.value, null, oldState.defaultValue, null);
|
||
|
|
||
|
updateMemoryUsagePerPackageLocked(oldState.packageName, newSize);
|
||
|
|
||
|
@@ -384,15 +390,16 @@ final class SettingsState {
|
||
|
}
|
||
|
|
||
|
Setting setting = mSettings.get(name);
|
||
|
+ if (setting == null) {
|
||
|
+ return false;
|
||
|
+ }
|
||
|
|
||
|
Setting oldSetting = new Setting(setting);
|
||
|
String oldValue = setting.getValue();
|
||
|
String oldDefaultValue = setting.getDefaultValue();
|
||
|
- String newValue = oldDefaultValue;
|
||
|
- String newDefaultValue = oldDefaultValue;
|
||
|
|
||
|
- int newSize = getNewMemoryUsagePerPackageLocked(setting.packageName, oldValue,
|
||
|
- newValue, oldDefaultValue, newDefaultValue);
|
||
|
+ int newSize = getNewMemoryUsagePerPackageLocked(setting.packageName, 0, oldValue,
|
||
|
+ oldDefaultValue, oldDefaultValue, oldDefaultValue);
|
||
|
checkNewMemoryUsagePerPackageLocked(setting.packageName, newSize);
|
||
|
|
||
|
if (!setting.reset()) {
|
||
|
@@ -521,8 +528,8 @@ final class SettingsState {
|
||
|
}
|
||
|
|
||
|
@GuardedBy("mLock")
|
||
|
- private int getNewMemoryUsagePerPackageLocked(String packageName, String oldValue,
|
||
|
- String newValue, String oldDefaultValue, String newDefaultValue) {
|
||
|
+ private int getNewMemoryUsagePerPackageLocked(String packageName, int deltaKeySize,
|
||
|
+ String oldValue, String newValue, String oldDefaultValue, String newDefaultValue) {
|
||
|
if (isExemptFromMemoryUsageCap(packageName)) {
|
||
|
return 0;
|
||
|
}
|
||
|
@@ -531,7 +538,7 @@ final class SettingsState {
|
||
|
final int newValueSize = (newValue != null) ? newValue.length() : 0;
|
||
|
final int oldDefaultValueSize = (oldDefaultValue != null) ? oldDefaultValue.length() : 0;
|
||
|
final int newDefaultValueSize = (newDefaultValue != null) ? newDefaultValue.length() : 0;
|
||
|
- final int deltaSize = newValueSize + newDefaultValueSize
|
||
|
+ final int deltaSize = deltaKeySize + newValueSize + newDefaultValueSize
|
||
|
- oldValueSize - oldDefaultValueSize;
|
||
|
return Math.max((currentSize != null) ? currentSize + deltaSize : deltaSize, 0);
|
||
|
}
|
||
|
@@ -1140,4 +1147,11 @@ final class SettingsState {
|
||
|
return false;
|
||
|
}
|
||
|
}
|
||
|
+
|
||
|
+ @VisibleForTesting
|
||
|
+ public int getMemoryUsage(String packageName) {
|
||
|
+ synchronized (mLock) {
|
||
|
+ return mPackageToMemoryUsage.getOrDefault(packageName, 0);
|
||
|
+ }
|
||
|
+ }
|
||
|
}
|
||
|
diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java
|
||
|
index 6f45adef91f7..adb356726eec 100644
|
||
|
--- a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java
|
||
|
+++ b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java
|
||
|
@@ -186,8 +186,8 @@ public class SettingsStateTest extends AndroidTestCase {
|
||
|
|
||
|
public void testInsertSetting_memoryUsage() {
|
||
|
final Object lock = new Object();
|
||
|
- final File file = new File(getContext().getCacheDir(), "setting.xml");
|
||
|
- final String settingName = "test_setting";
|
||
|
+ final File file = new File(getContext().getCacheDir(), "setting.xml");
|
||
|
+ final String settingName = "test_setting";
|
||
|
|
||
|
SettingsState settingsState = new SettingsState(getContext(), lock, file, 1,
|
||
|
SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper());
|
||
|
@@ -204,7 +204,7 @@ public class SettingsStateTest extends AndroidTestCase {
|
||
|
settingsState.deleteSettingLocked(settingName);
|
||
|
|
||
|
// Should not throw if usage is under the cap
|
||
|
- settingsState.insertSettingLocked(settingName, Strings.repeat("A", 19999),
|
||
|
+ settingsState.insertSettingLocked(settingName, Strings.repeat("A", 19975),
|
||
|
null, false, "p1");
|
||
|
settingsState.deleteSettingLocked(settingName);
|
||
|
try {
|
||
|
@@ -222,5 +222,101 @@ public class SettingsStateTest extends AndroidTestCase {
|
||
|
assertTrue(ex.getMessage().contains("p1"));
|
||
|
}
|
||
|
assertTrue(settingsState.getSettingLocked(settingName).isNull());
|
||
|
+ try {
|
||
|
+ settingsState.insertSettingLocked(Strings.repeat("A", 20001), "",
|
||
|
+ null, false, "p1");
|
||
|
+ fail("Should throw because it exceeded per package memory usage");
|
||
|
+ } catch (IllegalStateException ex) {
|
||
|
+ assertTrue(ex.getMessage().contains("You are adding too many system settings"));
|
||
|
+ }
|
||
|
+ }
|
||
|
+
|
||
|
+ public void testMemoryUsagePerPackage() {
|
||
|
+ final Object lock = new Object();
|
||
|
+ final File file = new File(getContext().getCacheDir(), "setting.xml");
|
||
|
+ final String testPackage = "package";
|
||
|
+ SettingsState settingsState = new SettingsState(getContext(), lock, file, 1,
|
||
|
+ SettingsState.MAX_BYTES_PER_APP_PACKAGE_LIMITED, Looper.getMainLooper());
|
||
|
+
|
||
|
+ // Test inserting one key with default
|
||
|
+ final String settingName = "test_setting";
|
||
|
+ final String testKey1 = settingName;
|
||
|
+ final String testValue1 = Strings.repeat("A", 100);
|
||
|
+ settingsState.insertSettingLocked(testKey1, testValue1, null, true, testPackage);
|
||
|
+ int expectedMemUsage = testKey1.length() + testValue1.length()
|
||
|
+ + testValue1.length() /* size for default */;
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+
|
||
|
+ // Test inserting another key
|
||
|
+ final String testKey2 = settingName + "2";
|
||
|
+ settingsState.insertSettingLocked(testKey2, testValue1, null, false, testPackage);
|
||
|
+ expectedMemUsage += testKey2.length() + testValue1.length();
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+
|
||
|
+ // Test updating first key with new default
|
||
|
+ final String testValue2 = Strings.repeat("A", 300);
|
||
|
+ settingsState.insertSettingLocked(testKey1, testValue2, null, true, testPackage);
|
||
|
+ expectedMemUsage += (testValue2.length() - testValue1.length()) * 2;
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+
|
||
|
+ // Test updating first key without new default
|
||
|
+ final String testValue3 = Strings.repeat("A", 50);
|
||
|
+ settingsState.insertSettingLocked(testKey1, testValue3, null, false, testPackage);
|
||
|
+ expectedMemUsage -= testValue2.length() - testValue3.length();
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+
|
||
|
+ // Test updating second key
|
||
|
+ settingsState.insertSettingLocked(testKey2, testValue2, null, false, testPackage);
|
||
|
+ expectedMemUsage -= testValue1.length() - testValue2.length();
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+
|
||
|
+ // Test resetting key
|
||
|
+ settingsState.resetSettingLocked(testKey1);
|
||
|
+ expectedMemUsage += testValue2.length() - testValue3.length();
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+
|
||
|
+ // Test resetting default value
|
||
|
+ settingsState.resetSettingDefaultValueLocked(testKey1);
|
||
|
+ expectedMemUsage -= testValue2.length();
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+
|
||
|
+ // Test deletion
|
||
|
+ settingsState.deleteSettingLocked(testKey2);
|
||
|
+ expectedMemUsage -= testValue2.length() + testKey2.length() /* key is deleted too */;
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+
|
||
|
+ // Test another package with a different key
|
||
|
+ final String testPackage2 = testPackage + "2";
|
||
|
+ final String testKey3 = settingName + "3";
|
||
|
+ settingsState.insertSettingLocked(testKey3, testValue1, null, true, testPackage2);
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+ final int expectedMemUsage2 = testKey3.length() + testValue1.length() * 2;
|
||
|
+ assertEquals(expectedMemUsage2, settingsState.getMemoryUsage(testPackage2));
|
||
|
+
|
||
|
+ // Test system package
|
||
|
+ settingsState.insertSettingLocked(testKey1, testValue1, null, true, "android");
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+ assertEquals(expectedMemUsage2, settingsState.getMemoryUsage(testPackage2));
|
||
|
+ assertEquals(0, settingsState.getMemoryUsage("android"));
|
||
|
+
|
||
|
+ // Test invalid value
|
||
|
+ try {
|
||
|
+ settingsState.insertSettingLocked(testKey1, Strings.repeat("A", 20001), null, false,
|
||
|
+ testPackage);
|
||
|
+ fail("Should throw because it exceeded per package memory usage");
|
||
|
+ } catch (IllegalStateException ex) {
|
||
|
+ assertTrue(ex.getMessage().contains("You are adding too many system settings"));
|
||
|
+ }
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
+
|
||
|
+ // Test invalid key
|
||
|
+ try {
|
||
|
+ settingsState.insertSettingLocked(Strings.repeat("A", 20001), "", null, false,
|
||
|
+ testPackage);
|
||
|
+ fail("Should throw because it exceeded per package memory usage");
|
||
|
+ } catch (IllegalStateException ex) {
|
||
|
+ assertTrue(ex.getMessage().contains("You are adding too many system settings"));
|
||
|
+ }
|
||
|
+ assertEquals(expectedMemUsage, settingsState.getMemoryUsage(testPackage));
|
||
|
}
|
||
|
}
|