From 62579b157a591b6aa464b28a5cabbffa2c190d8b Mon Sep 17 00:00:00 2001 From: Scott Mertz Date: Fri, 22 Jan 2016 14:36:19 -0800 Subject: [PATCH] cmsdk: add invalid input checking for persistent storage APIs Some of this exists in the PersistentStorage implementation, but it was never formally documented in the APIs. Inherit the cmhw implementation error checking & move it into the service. Add tests to validate the new restrictions & a test that was previously failing. Change-Id: I3ecda29fdd28bbc4e6d8ccce7511c4644065ea46 --- .../platform/internal/CMHardwareService.java | 14 ++++ .../hardware/CMHardwareManager.java | 16 ++--- .../hardware/unit/PersistentStorageTest.java | 70 +++++++++++++++++++ 3 files changed, 92 insertions(+), 8 deletions(-) diff --git a/cm/lib/main/java/org/cyanogenmod/platform/internal/CMHardwareService.java b/cm/lib/main/java/org/cyanogenmod/platform/internal/CMHardwareService.java index 9c3eed0..05ee6c7 100644 --- a/cm/lib/main/java/org/cyanogenmod/platform/internal/CMHardwareService.java +++ b/cm/lib/main/java/org/cyanogenmod/platform/internal/CMHardwareService.java @@ -32,6 +32,7 @@ import cyanogenmod.hardware.IThermalListenerCallback; import cyanogenmod.hardware.ThermalListenerCallback; import java.io.File; +import java.util.Arrays; import org.cyanogenmod.hardware.AdaptiveBacklight; import org.cyanogenmod.hardware.AutoContrast; @@ -592,6 +593,15 @@ public class CMHardwareService extends SystemService implements ThermalUpdateCal public boolean writePersistentBytes(String key, byte[] value) { mContext.enforceCallingOrSelfPermission( cyanogenmod.platform.Manifest.permission.MANAGE_PERSISTENT_STORAGE, null); + if (key == null || key.length() == 0 || key.length() > 64) { + Log.e(TAG, "Invalid key: " + key); + return false; + } + // A null value is delete + if (value != null && (value.length > 4096 || value.length == 0)) { + Log.e(TAG, "Invalid value: " + (value != null ? Arrays.toString(value) : null)); + return false; + } if (!isSupported(CMHardwareManager.FEATURE_PERSISTENT_STORAGE)) { Log.e(TAG, "Persistent storage is not supported"); return false; @@ -603,6 +613,10 @@ public class CMHardwareService extends SystemService implements ThermalUpdateCal public byte[] readPersistentBytes(String key) { mContext.enforceCallingOrSelfPermission( cyanogenmod.platform.Manifest.permission.MANAGE_PERSISTENT_STORAGE, null); + if (key == null || key.length() == 0 || key.length() > 64) { + Log.e(TAG, "Invalid key: " + key); + return null; + } if (!isSupported(CMHardwareManager.FEATURE_PERSISTENT_STORAGE)) { Log.e(TAG, "Persistent storage is not supported"); return null; diff --git a/src/java/cyanogenmod/hardware/CMHardwareManager.java b/src/java/cyanogenmod/hardware/CMHardwareManager.java index c28cb98..4425873 100644 --- a/src/java/cyanogenmod/hardware/CMHardwareManager.java +++ b/src/java/cyanogenmod/hardware/CMHardwareManager.java @@ -428,8 +428,8 @@ public final class CMHardwareManager { /** * Write a string to persistent storage, which persists thru factory reset * - * @param key String identifier for this item - * @param value The UTF-8 encoded string to store + * @param key String identifier for this item. Must not exceed 64 characters. + * @param value The UTF-8 encoded string to store of at least 1 character. null deletes the key/value pair. * @return true on success */ public boolean writePersistentString(String key, String value) { @@ -448,7 +448,7 @@ public final class CMHardwareManager { /** * Write an integer to persistent storage, which persists thru factory reset * - * @param key String identifier for this item + * @param key String identifier for this item. Must not exceed 64 characters. * @param value The integer to store * @return true on success */ @@ -466,8 +466,8 @@ public final class CMHardwareManager { /** * Write a byte array to persistent storage, which persists thru factory reset * - * @param key String identifier for this item - * @param value The byte array to store, up to 4096 bytes + * @param key String identifier for this item. Must not exceed 64 characters. + * @param value The byte array to store, must be 1-4096 bytes. null deletes the key/value pair. * @return true on success */ public boolean writePersistentBytes(String key, byte[] value) { @@ -483,7 +483,7 @@ public final class CMHardwareManager { /** * Read a string from persistent storage * - * @param key String identifier for this item + * @param key String identifier for this item. Must not exceed 64 characters. * @return the stored UTF-8 encoded string, null if not found */ public String readPersistentString(String key) { @@ -504,7 +504,7 @@ public final class CMHardwareManager { /** * Read an integer from persistent storage * - * @param key String identifier for this item + * @param key String identifier for this item. Must not exceed 64 characters. * @return the stored integer, zero if not found */ public int readPersistentInt(String key) { @@ -523,7 +523,7 @@ public final class CMHardwareManager { /** * Read a byte array from persistent storage * - * @param key String identifier for this item + * @param key String identifier for this item. Must not exceed 64 characters. * @return the stored byte array, null if not found */ public byte[] readPersistentBytes(String key) { diff --git a/tests/src/org/cyanogenmod/tests/hardware/unit/PersistentStorageTest.java b/tests/src/org/cyanogenmod/tests/hardware/unit/PersistentStorageTest.java index 7f7b215..10bc390 100644 --- a/tests/src/org/cyanogenmod/tests/hardware/unit/PersistentStorageTest.java +++ b/tests/src/org/cyanogenmod/tests/hardware/unit/PersistentStorageTest.java @@ -34,6 +34,58 @@ public class PersistentStorageTest extends AndroidTestCase { mHardwareManager = CMHardwareManager.getInstance(mContext); } + @SmallTest + public void testUdidFailure() { + String key = "udid"; + String value = "542bc67e510e82bd6d44e4f7015d7970"; + assertTrue(mHardwareManager.writePersistentString(key, value)); + } + + @SmallTest + public void testPersistentStringInvalidInput() { + String testKey = UUID.randomUUID().toString(); + String testString = "IM IN UR STORAGE"; + String testKeyTooLong = getStringOfLength(65); + String testStringTooLong = getStringOfLength(4097); + + assertFalse(mHardwareManager.writePersistentString(null, testString)); + assertFalse(mHardwareManager.writePersistentString("", testString)); + assertFalse(mHardwareManager.writePersistentString(testKeyTooLong, testString)); + assertFalse(mHardwareManager.writePersistentString(testKey, testStringTooLong)); + assertFalse(mHardwareManager.writePersistentString(testKey, "")); + assertNull(mHardwareManager.readPersistentString(testKey)); + assertNull(mHardwareManager.readPersistentString(testKeyTooLong)); + } + + @SmallTest + public void testPersistentIntInvalidInput() { + String testKey = UUID.randomUUID().toString(); + String testString = "IM IN UR STORAGE"; + String testKeyTooLong = getStringOfLength(65); + + assertFalse(mHardwareManager.writePersistentInt(null, 49152)); + assertFalse(mHardwareManager.writePersistentInt("", 49152)); + assertFalse(mHardwareManager.writePersistentInt(testKeyTooLong, 49152)); + assertEquals(0, mHardwareManager.readPersistentInt(testKey)); + assertEquals(0, mHardwareManager.readPersistentInt(testKeyTooLong)); + } + + @SmallTest + public void testPersistentBytesInvalidInput() { + String testKey = UUID.randomUUID().toString(); + byte[] testArray = new byte[1024]; + byte[] testArrayTooLong = new byte[4097]; + String testKeyTooLong = getStringOfLength(65); + + assertFalse(mHardwareManager.writePersistentBytes(null, testArray)); + assertFalse(mHardwareManager.writePersistentBytes("", testArray)); + assertFalse(mHardwareManager.writePersistentBytes(testKeyTooLong, testArray)); + assertFalse(mHardwareManager.writePersistentBytes(testKey, testArrayTooLong)); + assertFalse(mHardwareManager.writePersistentBytes(testKey, new byte[0])); + assertNull(mHardwareManager.readPersistentBytes(testKey)); + assertNull(mHardwareManager.readPersistentBytes(testKeyTooLong)); + } + @SmallTest public void testPersistentString() { assertTrue(mHardwareManager.isSupported(CMHardwareManager.FEATURE_PERSISTENT_STORAGE)); @@ -52,6 +104,12 @@ public class PersistentStorageTest extends AndroidTestCase { // erase + read assertTrue(mHardwareManager.deletePersistentObject(testKey)); assertNull(mHardwareManager.readPersistentString(testKey)); + + // erase through write null + assertTrue(mHardwareManager.writePersistentString(testKey, testString + " AGAIN")); + assertEquals(testString + " AGAIN", mHardwareManager.readPersistentString(testKey)); + assertTrue(mHardwareManager.writePersistentString(testKey, null)); + assertNull(mHardwareManager.readPersistentString(testKey)); } @SmallTest @@ -96,5 +154,17 @@ public class PersistentStorageTest extends AndroidTestCase { // erase + read assertTrue(mHardwareManager.deletePersistentObject(testKey)); assertNull(mHardwareManager.readPersistentBytes(testKey)); + + // erase through write null + assertTrue(mHardwareManager.writePersistentBytes(testKey, testArray)); + assertTrue(Arrays.equals(testArray, mHardwareManager.readPersistentBytes(testKey))); + assertTrue(mHardwareManager.writePersistentBytes(testKey, null)); + assertNull(mHardwareManager.readPersistentBytes(testKey)); + } + + private String getStringOfLength(int length) { + char[] chars = new char[length]; + Arrays.fill(chars, 'z'); + return new String(chars); } }