Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[SettingsProvider] mem limit should be checked before settings are updated

Browse Source 
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 8eeb92950f)
Merged-In: I20551a2dba9aa79efa0c064824f349f551c2c2e4
Change-Id: I20551a2dba9aa79efa0c064824f349f551c2c2e4
This commit is contained in:
Songchun Fan
2022-08-17 09:37:18 -07:00
parent 600ecb01d0
commit d85a428210
3 changed files with 96 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
packages/SettingsProvider/Android.bp
View File

@@ -24,7 +24,10 @@ android_test {
"src/com/android/providers/settings/SettingsState.java",
"src/com/android/providers/settings/SettingsHelper.java",
],
static_libs: ["androidx.test.rules"],
static_libs: [
"androidx.test.rules",
"truth-prebuilt",
],
libs: ["android.test.base"],
resource_dirs: ["res"],
aaptflags: [

79
packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
View File

@@ -367,9 +367,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();
}
}
@@ -392,6 +394,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) {
@@ -409,8 +417,7 @@ final class SettingsState {
addHistoricalOperationLocked(HISTORICAL_OPERATION_UPDATE, newState);
updateMemoryUsagePerPackageLocked(packageName, oldValue, value,
oldDefaultValue, newState.getDefaultValue());
updateMemoryUsagePerPackageLocked(packageName, newSize);
scheduleWriteIfNeededLocked();
@@ -431,13 +438,14 @@ final class SettingsState {
}
Setting oldState = mSettings.remove(name);
int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName, oldState.value,
null, oldState.defaultValue, null);
StatsLog.write(StatsLog.SETTING_CHANGED, name, /* value= */ "", /* newValue= */ "",
oldState.value, /* tag */ "", false, getUserIdFromKey(mKey),
StatsLog.SETTING_CHANGED__REASON__DELETED);
updateMemoryUsagePerPackageLocked(oldState.packageName, oldState.value,
null, oldState.defaultValue, null);
updateMemoryUsagePerPackageLocked(oldState.packageName, newSize);
addHistoricalOperationLocked(HISTORICAL_OPERATION_DELETE, oldState);
@@ -458,16 +466,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);
@@ -575,38 +585,49 @@ final class SettingsState {
}
@GuardedBy("mLock")
private void updateMemoryUsagePerPackageLocked(String packageName, String oldValue,
private boolean isExemptFromMemoryUsageCap(String packageName) {
return mMaxBytesPerAppPackage == MAX_BYTES_PER_APP_PACKAGE_UNLIMITED
|| 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 (mMaxBytesPerAppPackage == MAX_BYTES_PER_APP_PACKAGE_UNLIMITED) {
return;
if (isExemptFromMemoryUsageCap(packageName)) {
return 0;
}
if (SYSTEM_PACKAGE_NAME.equals(packageName)) {
return;
}
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);
}

43
packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java
View File

@@ -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());
}
}