mirror of
https://github.com/Divested-Mobile/DivestOS-Build.git
synced 2024-08-22 18:25:54 +00:00
235 lines
11 KiB
Diff
235 lines
11 KiB
Diff
![]() |
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||
|
From: Songchun Fan <schfan@google.com>
|
||
|
Date: Wed, 17 Aug 2022 09:37:18 -0700
|
||
|
Subject: [PATCH] mem limit should be checked before settings are updated
|
||
|
|
||
|
Previously, a setting is updated before the memory usage limit
|
||
|
check, which can be exploited by malicious apps and cause OoM DoS.
|
||
|
|
||
|
This CL changes the logic to checkMemLimit -> update -> updateMemUsage.
|
||
|
|
||
|
BUG: 239415861
|
||
|
Test: atest com.android.providers.settings.SettingsStateTest
|
||
|
|
||
|
(cherry picked from commit 8eeb92950f4a7012d4cf282106a1418fd211f475)
|
||
|
Merged-In: I20551a2dba9aa79efa0c064824f349f551c2c2e4
|
||
|
Change-Id: I20551a2dba9aa79efa0c064824f349f551c2c2e4
|
||
|
(cherry picked from commit d85a42821075ad80b931d904bdc9c1d4c3129456)
|
||
|
Merged-In: I20551a2dba9aa79efa0c064824f349f551c2c2e4
|
||
|
---
|
||
|
.../providers/settings/SettingsState.java | 75 ++++++++++++-------
|
||
|
.../providers/settings/SettingsStateTest.java | 43 ++++++++++-
|
||
|
2 files changed, 90 insertions(+), 28 deletions(-)
|
||
|
|
||
|
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
|
||
|
index 4151ada9ce80..0e94ea85919b 100644
|
||
|
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
|
||
|
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
|
||
|
@@ -306,9 +306,11 @@ final class SettingsState {
|
||
|
Setting newSetting = new Setting(name, oldSetting.getValue(), null,
|
||
|
oldSetting.getPackageName(), oldSetting.getTag(), false,
|
||
|
oldSetting.getId());
|
||
|
- mSettings.put(name, newSetting);
|
||
|
- updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue,
|
||
|
+ int newSize = getNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue,
|
||
|
newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue());
|
||
|
+ checkNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize);
|
||
|
+ mSettings.put(name, newSetting);
|
||
|
+ updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize);
|
||
|
scheduleWriteIfNeededLocked();
|
||
|
}
|
||
|
}
|
||
|
@@ -323,6 +325,12 @@ final class SettingsState {
|
||
|
Setting oldState = mSettings.get(name);
|
||
|
String oldValue = (oldState != null) ? oldState.value : null;
|
||
|
String oldDefaultValue = (oldState != null) ? oldState.defaultValue : null;
|
||
|
+ String newDefaultValue = makeDefault ? value : oldDefaultValue;
|
||
|
+
|
||
|
+ int newSize = getNewMemoryUsagePerPackageLocked(packageName, oldValue, value,
|
||
|
+ oldDefaultValue, newDefaultValue);
|
||
|
+ checkNewMemoryUsagePerPackageLocked(packageName, newSize);
|
||
|
+
|
||
|
Setting newState;
|
||
|
|
||
|
if (oldState != null) {
|
||
|
@@ -337,8 +345,7 @@ final class SettingsState {
|
||
|
|
||
|
addHistoricalOperationLocked(HISTORICAL_OPERATION_UPDATE, newState);
|
||
|
|
||
|
- updateMemoryUsagePerPackageLocked(packageName, oldValue, value,
|
||
|
- oldDefaultValue, newState.getDefaultValue());
|
||
|
+ updateMemoryUsagePerPackageLocked(packageName, newSize);
|
||
|
|
||
|
scheduleWriteIfNeededLocked();
|
||
|
|
||
|
@@ -358,10 +365,11 @@ final class SettingsState {
|
||
|
}
|
||
|
|
||
|
Setting oldState = mSettings.remove(name);
|
||
|
-
|
||
|
- updateMemoryUsagePerPackageLocked(oldState.packageName, oldState.value,
|
||
|
+ int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName, oldState.value,
|
||
|
null, oldState.defaultValue, null);
|
||
|
|
||
|
+ updateMemoryUsagePerPackageLocked(oldState.packageName, newSize);
|
||
|
+
|
||
|
addHistoricalOperationLocked(HISTORICAL_OPERATION_DELETE, oldState);
|
||
|
|
||
|
scheduleWriteIfNeededLocked();
|
||
|
@@ -380,16 +388,18 @@ final class SettingsState {
|
||
|
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);
|
||
|
+ checkNewMemoryUsagePerPackageLocked(setting.packageName, newSize);
|
||
|
|
||
|
if (!setting.reset()) {
|
||
|
return false;
|
||
|
}
|
||
|
|
||
|
- String newValue = setting.getValue();
|
||
|
- String newDefaultValue = setting.getDefaultValue();
|
||
|
-
|
||
|
- updateMemoryUsagePerPackageLocked(setting.packageName, oldValue,
|
||
|
- newValue, oldDefaultValue, newDefaultValue);
|
||
|
+ updateMemoryUsagePerPackageLocked(setting.packageName, newSize);
|
||
|
|
||
|
addHistoricalOperationLocked(HISTORICAL_OPERATION_RESET, oldSetting);
|
||
|
|
||
|
@@ -492,38 +502,49 @@ final class SettingsState {
|
||
|
}
|
||
|
}
|
||
|
|
||
|
- private void updateMemoryUsagePerPackageLocked(String packageName, String oldValue,
|
||
|
- String newValue, String oldDefaultValue, String newDefaultValue) {
|
||
|
- if (mMaxBytesPerAppPackage == MAX_BYTES_PER_APP_PACKAGE_UNLIMITED) {
|
||
|
- return;
|
||
|
- }
|
||
|
+ private boolean isExemptFromMemoryUsageCap(String packageName) {
|
||
|
+ return mMaxBytesPerAppPackage == MAX_BYTES_PER_APP_PACKAGE_UNLIMITED
|
||
|
+ || SYSTEM_PACKAGE_NAME.equals(packageName);
|
||
|
+ }
|
||
|
|
||
|
- if (SYSTEM_PACKAGE_NAME.equals(packageName)) {
|
||
|
+ @GuardedBy("mLock")
|
||
|
+ private void checkNewMemoryUsagePerPackageLocked(String packageName, int newSize)
|
||
|
+ throws IllegalStateException {
|
||
|
+ if (isExemptFromMemoryUsageCap(packageName)) {
|
||
|
return;
|
||
|
}
|
||
|
+ if (newSize > mMaxBytesPerAppPackage) {
|
||
|
+ throw new IllegalStateException("You are adding too many system settings. "
|
||
|
+ + "You should stop using system settings for app specific data"
|
||
|
+ + " package: " + packageName);
|
||
|
+ }
|
||
|
+ }
|
||
|
|
||
|
+ @GuardedBy("mLock")
|
||
|
+ private int getNewMemoryUsagePerPackageLocked(String packageName, String oldValue,
|
||
|
+ String newValue, String oldDefaultValue, String newDefaultValue) {
|
||
|
+ if (isExemptFromMemoryUsageCap(packageName)) {
|
||
|
+ return 0;
|
||
|
+ }
|
||
|
+ final Integer currentSize = mPackageToMemoryUsage.get(packageName);
|
||
|
final int oldValueSize = (oldValue != null) ? oldValue.length() : 0;
|
||
|
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
|
||
|
- oldValueSize - oldDefaultValueSize;
|
||
|
+ return Math.max((currentSize != null) ? currentSize + deltaSize : deltaSize, 0);
|
||
|
+ }
|
||
|
|
||
|
- Integer currentSize = mPackageToMemoryUsage.get(packageName);
|
||
|
- final int newSize = Math.max((currentSize != null)
|
||
|
- ? currentSize + deltaSize : deltaSize, 0);
|
||
|
-
|
||
|
- if (newSize > mMaxBytesPerAppPackage) {
|
||
|
- throw new IllegalStateException("You are adding too many system settings. "
|
||
|
- + "You should stop using system settings for app specific data"
|
||
|
- + " package: " + packageName);
|
||
|
+ @GuardedBy("mLock")
|
||
|
+ private void updateMemoryUsagePerPackageLocked(String packageName, int newSize) {
|
||
|
+ if (isExemptFromMemoryUsageCap(packageName)) {
|
||
|
+ return;
|
||
|
}
|
||
|
-
|
||
|
if (DEBUG) {
|
||
|
Slog.i(LOG_TAG, "Settings for package: " + packageName
|
||
|
+ " size: " + newSize + " bytes.");
|
||
|
}
|
||
|
-
|
||
|
mPackageToMemoryUsage.put(packageName, newSize);
|
||
|
}
|
||
|
|
||
|
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 3f68554ffe87..6f45adef91f7 100644
|
||
|
--- a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java
|
||
|
+++ b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java
|
||
|
@@ -21,6 +21,8 @@ import android.util.Xml;
|
||
|
|
||
|
import org.xmlpull.v1.XmlSerializer;
|
||
|
|
||
|
+import com.google.common.base.Strings;
|
||
|
+
|
||
|
import java.io.ByteArrayOutputStream;
|
||
|
import java.io.File;
|
||
|
import java.io.FileOutputStream;
|
||
|
@@ -46,7 +48,6 @@ public class SettingsStateTest extends AndroidTestCase {
|
||
|
"\uD800ab\uDC00 " + // broken surrogate pairs
|
||
|
"日本語";
|
||
|
|
||
|
-
|
||
|
public void testIsBinary() {
|
||
|
assertFalse(SettingsState.isBinary(" abc 日本語"));
|
||
|
|
||
|
@@ -182,4 +183,44 @@ public class SettingsStateTest extends AndroidTestCase {
|
||
|
assertEquals("p2", s.getPackageName());
|
||
|
}
|
||
|
}
|
||
|
+
|
||
|
+ public void testInsertSetting_memoryUsage() {
|
||
|
+ final Object lock = new Object();
|
||
|
+ 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());
|
||
|
+ // No exception should be thrown when there is no cap
|
||
|
+ settingsState.insertSettingLocked(settingName, Strings.repeat("A", 20001),
|
||
|
+ null, false, "p1");
|
||
|
+ settingsState.deleteSettingLocked(settingName);
|
||
|
+
|
||
|
+ settingsState = new SettingsState(getContext(), lock, file, 1,
|
||
|
+ SettingsState.MAX_BYTES_PER_APP_PACKAGE_LIMITED, Looper.getMainLooper());
|
||
|
+ // System package doesn't have memory usage limit
|
||
|
+ settingsState.insertSettingLocked(settingName, Strings.repeat("A", 20001),
|
||
|
+ null, false, "android");
|
||
|
+ settingsState.deleteSettingLocked(settingName);
|
||
|
+
|
||
|
+ // Should not throw if usage is under the cap
|
||
|
+ settingsState.insertSettingLocked(settingName, Strings.repeat("A", 19999),
|
||
|
+ null, false, "p1");
|
||
|
+ settingsState.deleteSettingLocked(settingName);
|
||
|
+ try {
|
||
|
+ settingsState.insertSettingLocked(settingName, Strings.repeat("A", 20001),
|
||
|
+ null, false, "p1");
|
||
|
+ fail("Should throw because it exceeded per package memory usage");
|
||
|
+ } catch (IllegalStateException ex) {
|
||
|
+ assertTrue(ex.getMessage().contains("p1"));
|
||
|
+ }
|
||
|
+ try {
|
||
|
+ settingsState.insertSettingLocked(settingName, Strings.repeat("A", 20001),
|
||
|
+ null, false, "p1");
|
||
|
+ fail("Should throw because it exceeded per package memory usage");
|
||
|
+ } catch (IllegalStateException ex) {
|
||
|
+ assertTrue(ex.getMessage().contains("p1"));
|
||
|
+ }
|
||
|
+ assertTrue(settingsState.getSettingLocked(settingName).isNull());
|
||
|
+ }
|
||
|
}
|