From 72b8e58db8db88881fa7b4c723557ae3fba4c9f7 Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Mon, 12 Jun 2023 15:12:19 -0700 Subject: [PATCH] Assert type of 16-bit value in MultiStateCounter parcelables Bug: 285143726 Test: atest FrameworksCoreTests:LongMultiStateCounterTest FrameworksCoreTests:LongArrayMultiStateCounterTest Change-Id: Ia3708bbf6553fd1e7a1ad22aaccbaa4d12b9c23a (cherry picked from commit 6a110d9c4c2414ff8b4cb5f13a2ecf0a6b93cb96) --- ...internal_os_LongArrayMultiStateCounter.cpp | 12 ++++-- ...roid_internal_os_LongMultiStateCounter.cpp | 12 ++++-- .../os/LongArrayMultiStateCounterTest.java | 40 +++++++++++++++++++ .../os/LongMultiStateCounterTest.java | 39 ++++++++++++++++++ 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/core/jni/com_android_internal_os_LongArrayMultiStateCounter.cpp b/core/jni/com_android_internal_os_LongArrayMultiStateCounter.cpp index a95b6e37f5de2..76f5c107c970d 100644 --- a/core/jni/com_android_internal_os_LongArrayMultiStateCounter.cpp +++ b/core/jni/com_android_internal_os_LongArrayMultiStateCounter.cpp @@ -127,16 +127,17 @@ static void native_writeToParcel(JNIEnv *env, jobject self, jlong nativePtr, job } } -static void throwReadRE(JNIEnv *env, binder_status_t status) { +static void throwReadException(JNIEnv *env, binder_status_t status) { ALOGE("Could not read LongArrayMultiStateCounter from Parcel, status = %d", status); - jniThrowRuntimeException(env, "Could not read LongArrayMultiStateCounter from Parcel"); + jniThrowException(env, "android.os.BadParcelableException", + "Could not read LongArrayMultiStateCounter from Parcel"); } #define THROW_AND_RETURN_ON_READ_ERROR(expr) \ { \ binder_status_t status = expr; \ if (status != STATUS_OK) { \ - throwReadRE(env, status); \ + throwReadException(env, status); \ return 0L; \ } \ } @@ -147,6 +148,11 @@ static jlong native_initFromParcel(JNIEnv *env, jclass theClass, jobject jParcel int32_t stateCount; THROW_AND_RETURN_ON_READ_ERROR(AParcel_readInt32(parcel.get(), &stateCount)); + if (stateCount < 0 || stateCount > 0xEFFF) { + throwReadException(env, STATUS_INVALID_OPERATION); + return 0L; + } + int32_t arrayLength; THROW_AND_RETURN_ON_READ_ERROR(AParcel_readInt32(parcel.get(), &arrayLength)); diff --git a/core/jni/com_android_internal_os_LongMultiStateCounter.cpp b/core/jni/com_android_internal_os_LongMultiStateCounter.cpp index 1712b3a8512b7..ddf7a67e00cef 100644 --- a/core/jni/com_android_internal_os_LongMultiStateCounter.cpp +++ b/core/jni/com_android_internal_os_LongMultiStateCounter.cpp @@ -131,16 +131,17 @@ static void native_writeToParcel(JNIEnv *env, jobject self, jlong nativePtr, job } } -static void throwReadRE(JNIEnv *env, binder_status_t status) { +static void throwReadException(JNIEnv *env, binder_status_t status) { ALOGE("Could not read LongMultiStateCounter from Parcel, status = %d", status); - jniThrowRuntimeException(env, "Could not read LongMultiStateCounter from Parcel"); + jniThrowException(env, "android.os.BadParcelableException", + "Could not read LongMultiStateCounter from Parcel"); } #define THROW_AND_RETURN_ON_READ_ERROR(expr) \ { \ binder_status_t status = expr; \ if (status != STATUS_OK) { \ - throwReadRE(env, status); \ + throwReadException(env, status); \ return 0L; \ } \ } @@ -151,6 +152,11 @@ static jlong native_initFromParcel(JNIEnv *env, jclass theClass, jobject jParcel int32_t stateCount; THROW_AND_RETURN_ON_READ_ERROR(AParcel_readInt32(parcel.get(), &stateCount)); + if (stateCount < 0 || stateCount > 0xEFFF) { + throwReadException(env, STATUS_INVALID_OPERATION); + return 0L; + } + auto counter = std::make_unique(stateCount, 0); for (battery::state_t state = 0; state < stateCount; state++) { diff --git a/core/tests/coretests/src/com/android/internal/os/LongArrayMultiStateCounterTest.java b/core/tests/coretests/src/com/android/internal/os/LongArrayMultiStateCounterTest.java index 516dee7dc9aa3..faccf1ad19a10 100644 --- a/core/tests/coretests/src/com/android/internal/os/LongArrayMultiStateCounterTest.java +++ b/core/tests/coretests/src/com/android/internal/os/LongArrayMultiStateCounterTest.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; +import android.os.BadParcelableException; import android.os.Parcel; import androidx.test.filters.SmallTest; @@ -162,6 +163,45 @@ public class LongArrayMultiStateCounterTest { () -> LongArrayMultiStateCounter.CREATOR.createFromParcel(parcel)); } + @Test + public void createFromBadBundle() { + Parcel data = Parcel.obtain(); + int bundleLenPos = data.dataPosition(); + data.writeInt(0); + data.writeInt(0x4C444E42); // BaseBundle.BUNDLE_MAGIC + + int bundleStart = data.dataPosition(); + + data.writeInt(1); + data.writeString("key"); + data.writeInt(4); + int lazyValueLenPos = data.dataPosition(); + data.writeInt(0); + int lazyValueStart = data.dataPosition(); + data.writeString("com.android.internal.os.LongArrayMultiStateCounter"); + + // Invalid int16 value + data.writeInt(0x10000); // stateCount + data.writeInt(10); // arrayLength + for (int i = 0; i < 0x10000; ++i) { + data.writeLong(0); + } + + backPatchLength(data, lazyValueLenPos, lazyValueStart); + backPatchLength(data, bundleLenPos, bundleStart); + data.setDataPosition(0); + + assertThrows(BadParcelableException.class, + () -> data.readBundle().getParcelable("key", LongArrayMultiStateCounter.class)); + } + + private static void backPatchLength(Parcel parcel, int lengthPos, int startPos) { + int endPos = parcel.dataPosition(); + parcel.setDataPosition(lengthPos); + parcel.writeInt(endPos - startPos); + parcel.setDataPosition(endPos); + } + @Test public void combineValues() { long[] values = new long[] {0, 1, 2, 3, 42}; diff --git a/core/tests/coretests/src/com/android/internal/os/LongMultiStateCounterTest.java b/core/tests/coretests/src/com/android/internal/os/LongMultiStateCounterTest.java index fc86ebe1c10e4..341375357902e 100644 --- a/core/tests/coretests/src/com/android/internal/os/LongMultiStateCounterTest.java +++ b/core/tests/coretests/src/com/android/internal/os/LongMultiStateCounterTest.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; +import android.os.BadParcelableException; import android.os.Parcel; import androidx.test.filters.SmallTest; @@ -210,4 +211,42 @@ public class LongMultiStateCounterTest { assertThrows(RuntimeException.class, () -> LongMultiStateCounter.CREATOR.createFromParcel(parcel)); } + + @Test + public void createFromBadBundle() { + Parcel data = Parcel.obtain(); + int bundleLenPos = data.dataPosition(); + data.writeInt(0); + data.writeInt(0x4C444E42); // BaseBundle.BUNDLE_MAGIC + + int bundleStart = data.dataPosition(); + + data.writeInt(1); + data.writeString("key"); + data.writeInt(4); + int lazyValueLenPos = data.dataPosition(); + data.writeInt(0); + int lazyValueStart = data.dataPosition(); + data.writeString("com.android.internal.os.LongMultiStateCounter"); + + // Invalid int16 value + data.writeInt(0x10000); // stateCount + for (int i = 0; i < 0x10000; ++i) { + data.writeLong(0); + } + + backPatchLength(data, lazyValueLenPos, lazyValueStart); + backPatchLength(data, bundleLenPos, bundleStart); + data.setDataPosition(0); + + assertThrows(BadParcelableException.class, + () -> data.readBundle().getParcelable("key", LongMultiStateCounter.class)); + } + + private static void backPatchLength(Parcel parcel, int lengthPos, int startPos) { + int endPos = parcel.dataPosition(); + parcel.setDataPosition(lengthPos); + parcel.writeInt(endPos - startPos); + parcel.setDataPosition(endPos); + } }