Skip to content

Commit fd3e1a6

Browse files
schfan-1OhMyVenyx
schfan-1
authored and
OhMyVenyx
committed
[RESTRICT AUTOMERGE][SettingsProvider] 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 783bcba) (cherry picked from commit 0123e87) Merged-In: Ic9e88c0cc3d7206c64ba5b5c7d15b50d1ffc9adc
1 parent 2dc4541 commit fd3e1a6

2 files changed

Lines changed: 120 additions & 14 deletions

File tree

‎packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import android.util.proto.ProtoOutputStream;
4848

4949
import com.android.internal.annotations.GuardedBy;
50+
import com.android.internal.annotations.VisibleForTesting;
5051
import com.android.internal.util.FrameworkStatsLog;
5152

5253
import libcore.io.IoUtils;
@@ -375,8 +376,8 @@ public void resetSettingDefaultValueLocked(String name) {
375376
Setting newSetting = new Setting(name, oldSetting.getValue(), null,
376377
oldSetting.getPackageName(), oldSetting.getTag(), false,
377378
oldSetting.getId());
378-
int newSize = getNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue,
379-
newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue());
379+
int newSize = getNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), 0,
380+
oldValue, newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue());
380381
checkNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize);
381382
mSettings.put(name, newSetting);
382383
updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize);
@@ -414,8 +415,9 @@ public boolean insertSettingLocked(String name, String value, String tag,
414415
String oldDefaultValue = (oldState != null) ? oldState.defaultValue : null;
415416
String newDefaultValue = makeDefault ? value : oldDefaultValue;
416417

417-
int newSize = getNewMemoryUsagePerPackageLocked(packageName, oldValue, value,
418-
oldDefaultValue, newDefaultValue);
418+
int newSize = getNewMemoryUsagePerPackageLocked(packageName,
419+
oldValue == null ? name.length() : 0 /* deltaKeySize */,
420+
oldValue, value, oldDefaultValue, newDefaultValue);
419421
checkNewMemoryUsagePerPackageLocked(packageName, newSize);
420422

421423
Setting newState;
@@ -559,8 +561,12 @@ public boolean deleteSettingLocked(String name) {
559561
}
560562

561563
Setting oldState = mSettings.remove(name);
562-
int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName, oldState.value,
563-
null, oldState.defaultValue, null);
564+
if (oldState == null) {
565+
return false;
566+
}
567+
int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName,
568+
-name.length() /* deltaKeySize */,
569+
oldState.value, null, oldState.defaultValue, null);
564570

565571
FrameworkStatsLog.write(FrameworkStatsLog.SETTING_CHANGED, name, /* value= */ "",
566572
/* newValue= */ "", oldState.value, /* tag */ "", false, getUserIdFromKey(mKey),
@@ -583,15 +589,16 @@ public boolean resetSettingLocked(String name) {
583589
}
584590

585591
Setting setting = mSettings.get(name);
592+
if (setting == null) {
593+
return false;
594+
}
586595

587596
Setting oldSetting = new Setting(setting);
588597
String oldValue = setting.getValue();
589598
String oldDefaultValue = setting.getDefaultValue();
590-
String newValue = oldDefaultValue;
591-
String newDefaultValue = oldDefaultValue;
592599

593-
int newSize = getNewMemoryUsagePerPackageLocked(setting.packageName, oldValue,
594-
newValue, oldDefaultValue, newDefaultValue);
600+
int newSize = getNewMemoryUsagePerPackageLocked(setting.packageName, 0, oldValue,
601+
oldDefaultValue, oldDefaultValue, oldDefaultValue);
595602
checkNewMemoryUsagePerPackageLocked(setting.packageName, newSize);
596603

597604
if (!setting.reset()) {
@@ -725,8 +732,8 @@ private void checkNewMemoryUsagePerPackageLocked(String packageName, int newSize
725732
}
726733

727734
@GuardedBy("mLock")
728-
private int getNewMemoryUsagePerPackageLocked(String packageName, String oldValue,
729-
String newValue, String oldDefaultValue, String newDefaultValue) {
735+
private int getNewMemoryUsagePerPackageLocked(String packageName, int deltaKeySize,
736+
String oldValue, String newValue, String oldDefaultValue, String newDefaultValue) {
730737
if (isExemptFromMemoryUsageCap(packageName)) {
731738
return 0;
732739
}
@@ -735,7 +742,7 @@ private int getNewMemoryUsagePerPackageLocked(String packageName, String oldValu
735742
final int newValueSize = (newValue != null) ? newValue.length() : 0;
736743
final int oldDefaultValueSize = (oldDefaultValue != null) ? oldDefaultValue.length() : 0;
737744
final int newDefaultValueSize = (newDefaultValue != null) ? newDefaultValue.length() : 0;
738-
final int deltaSize = newValueSize + newDefaultValueSize
745+
final int deltaSize = deltaKeySize + newValueSize + newDefaultValueSize
739746
- oldValueSize - oldDefaultValueSize;
740747
return Math.max((currentSize != null) ? currentSize + deltaSize : deltaSize, 0);
741748
}
@@ -1556,4 +1563,11 @@ private static boolean isSystemPackage(@Nullable ApplicationInfo aInfo) {
15561563
}
15571564
return false;
15581565
}
1566+
1567+
@VisibleForTesting
1568+
public int getMemoryUsage(String packageName) {
1569+
synchronized (mLock) {
1570+
return mPackageToMemoryUsage.getOrDefault(packageName, 0);
1571+
}
1572+
}
15591573
}

‎packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java‎

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ public void testInsertSetting_memoryUsage() {
295295
settingsState.deleteSettingLocked(SETTING_NAME);
296296

297297
// Should not throw if usage is under the cap
298-
settingsState.insertSettingLocked(SETTING_NAME, Strings.repeat("A", 19999),
298+
settingsState.insertSettingLocked(SETTING_NAME, Strings.repeat("A", 19975),
299299
null, false, "p1");
300300
settingsState.deleteSettingLocked(SETTING_NAME);
301301
try {
@@ -313,5 +313,97 @@ public void testInsertSetting_memoryUsage() {
313313
assertTrue(ex.getMessage().contains("p1"));
314314
}
315315
assertTrue(settingsState.getSettingLocked(SETTING_NAME).isNull());
316+
try {
317+
settingsState.insertSettingLocked(Strings.repeat("A", 20001), "",
318+
null, false, "p1");
319+
fail("Should throw because it exceeded per package memory usage");
320+
} catch (IllegalStateException ex) {
321+
assertTrue(ex.getMessage().contains("You are adding too many system settings"));
322+
}
323+
}
324+
325+
public void testMemoryUsagePerPackage() {
326+
SettingsState settingsState = new SettingsState(getContext(), mLock, mSettingsFile, 1,
327+
SettingsState.MAX_BYTES_PER_APP_PACKAGE_LIMITED, Looper.getMainLooper());
328+
329+
// Test inserting one key with default
330+
final String testKey1 = SETTING_NAME;
331+
final String testValue1 = Strings.repeat("A", 100);
332+
settingsState.insertSettingLocked(testKey1, testValue1, null, true, TEST_PACKAGE);
333+
int expectedMemUsage = testKey1.length() + testValue1.length()
334+
+ testValue1.length() /* size for default */;
335+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
336+
337+
// Test inserting another key
338+
final String testKey2 = SETTING_NAME + "2";
339+
settingsState.insertSettingLocked(testKey2, testValue1, null, false, TEST_PACKAGE);
340+
expectedMemUsage += testKey2.length() + testValue1.length();
341+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
342+
343+
// Test updating first key with new default
344+
final String testValue2 = Strings.repeat("A", 300);
345+
settingsState.insertSettingLocked(testKey1, testValue2, null, true, TEST_PACKAGE);
346+
expectedMemUsage += (testValue2.length() - testValue1.length()) * 2;
347+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
348+
349+
// Test updating first key without new default
350+
final String testValue3 = Strings.repeat("A", 50);
351+
settingsState.insertSettingLocked(testKey1, testValue3, null, false, TEST_PACKAGE);
352+
expectedMemUsage -= testValue2.length() - testValue3.length();
353+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
354+
355+
// Test updating second key
356+
settingsState.insertSettingLocked(testKey2, testValue2, null, false, TEST_PACKAGE);
357+
expectedMemUsage -= testValue1.length() - testValue2.length();
358+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
359+
360+
// Test resetting key
361+
settingsState.resetSettingLocked(testKey1);
362+
expectedMemUsage += testValue2.length() - testValue3.length();
363+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
364+
365+
// Test resetting default value
366+
settingsState.resetSettingDefaultValueLocked(testKey1);
367+
expectedMemUsage -= testValue2.length();
368+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
369+
370+
// Test deletion
371+
settingsState.deleteSettingLocked(testKey2);
372+
expectedMemUsage -= testValue2.length() + testKey2.length() /* key is deleted too */;
373+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
374+
375+
// Test another package with a different key
376+
final String testPackage2 = TEST_PACKAGE + "2";
377+
final String testKey3 = SETTING_NAME + "3";
378+
settingsState.insertSettingLocked(testKey3, testValue1, null, true, testPackage2);
379+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
380+
final int expectedMemUsage2 = testKey3.length() + testValue1.length() * 2;
381+
assertEquals(expectedMemUsage2, settingsState.getMemoryUsage(testPackage2));
382+
383+
// Test system package
384+
settingsState.insertSettingLocked(testKey1, testValue1, null, true, SYSTEM_PACKAGE);
385+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
386+
assertEquals(expectedMemUsage2, settingsState.getMemoryUsage(testPackage2));
387+
assertEquals(0, settingsState.getMemoryUsage(SYSTEM_PACKAGE));
388+
389+
// Test invalid value
390+
try {
391+
settingsState.insertSettingLocked(testKey1, Strings.repeat("A", 20001), null, false,
392+
TEST_PACKAGE);
393+
fail("Should throw because it exceeded per package memory usage");
394+
} catch (IllegalStateException ex) {
395+
assertTrue(ex.getMessage().contains("You are adding too many system settings"));
396+
}
397+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
398+
399+
// Test invalid key
400+
try {
401+
settingsState.insertSettingLocked(Strings.repeat("A", 20001), "", null, false,
402+
TEST_PACKAGE);
403+
fail("Should throw because it exceeded per package memory usage");
404+
} catch (IllegalStateException ex) {
405+
assertTrue(ex.getMessage().contains("You are adding too many system settings"));
406+
}
407+
assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
316408
}
317409
}

0 commit comments

Comments
 (0)