From 7c00639a6802a9235ca9f2ad21577515215a4ee6 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Tue, 18 Apr 2017 15:54:43 -0700 Subject: [PATCH] Validate incoming data properly. Make sure calls to readBuffer() and readEmbeddedBuffer() get the correct size, parent and offset passed in, so these can be validated by libhwbinder. Modified HwBlob to take a length argument as well, so it can be validated. Bug: 30498700 Test: hidl_test, hidl_test_java, Youtube, Maps, Netflix, Camera Change-Id: I28712db97ae29b46acfe952d3d92d1ce5f666a4d Merged-In: I28712db97ae29b46acfe952d3d92d1ce5f666a4d --- core/java/android/os/HwParcel.java | 5 +-- core/jni/android_os_HwParcel.cpp | 49 ++++++++++++++++++------------ 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/core/java/android/os/HwParcel.java b/core/java/android/os/HwParcel.java index 94fd5b074b829..4ba1144742db0 100644 --- a/core/java/android/os/HwParcel.java +++ b/core/java/android/os/HwParcel.java @@ -209,10 +209,11 @@ public class HwParcel { public native final IHwBinder readStrongBinder(); // Handle is stored as part of the blob. - public native final HwBlob readBuffer(); + public native final HwBlob readBuffer(long expectedSize); public native final HwBlob readEmbeddedBuffer( - long parentHandle, long offset, boolean nullable); + long expectedSize, long parentHandle, long offset, + boolean nullable); public native final void writeBuffer(HwBlob blob); diff --git a/core/jni/android_os_HwParcel.cpp b/core/jni/android_os_HwParcel.cpp index 678041f85d173..b21ea828f2a4e 100644 --- a/core/jni/android_os_HwParcel.cpp +++ b/core/jni/android_os_HwParcel.cpp @@ -574,7 +574,7 @@ static jstring JHwParcel_native_readString(JNIEnv *env, jobject thiz) { size_t parentHandle; const hidl_string *s; - status_t err = parcel->readBuffer(&parentHandle, + status_t err = parcel->readBuffer(sizeof(*s), &parentHandle, reinterpret_cast(&s)); if (err != OK) { @@ -583,7 +583,7 @@ static jstring JHwParcel_native_readString(JNIEnv *env, jobject thiz) { } err = ::android::hardware::readEmbeddedFromParcel( - const_cast(s), + const_cast(*s), *parcel, parentHandle, 0 /* parentOffset */); if (err != OK) { @@ -602,7 +602,7 @@ static Type ## Array JHwParcel_native_read ## Suffix ## Vector( \ size_t parentHandle; \ \ const hidl_vec *vec; \ - status_t err = parcel->readBuffer(&parentHandle, \ + status_t err = parcel->readBuffer(sizeof(*vec), &parentHandle, \ reinterpret_cast(&vec)); \ \ if (err != OK) { \ @@ -613,7 +613,7 @@ static Type ## Array JHwParcel_native_read ## Suffix ## Vector( \ size_t childHandle; \ \ err = ::android::hardware::readEmbeddedFromParcel( \ - const_cast *>(vec), \ + const_cast &>(*vec), \ *parcel, \ parentHandle, \ 0 /* parentOffset */, \ @@ -645,7 +645,7 @@ static jbooleanArray JHwParcel_native_readBoolVector( size_t parentHandle; const hidl_vec *vec; - status_t err = parcel->readBuffer(&parentHandle, + status_t err = parcel->readBuffer(sizeof(*vec), &parentHandle, reinterpret_cast(&vec)); if (err != OK) { @@ -656,7 +656,7 @@ static jbooleanArray JHwParcel_native_readBoolVector( size_t childHandle; err = ::android::hardware::readEmbeddedFromParcel( - const_cast *>(vec), + const_cast &>(*vec), *parcel, parentHandle, 0 /* parentOffset */, @@ -709,7 +709,7 @@ static jobjectArray JHwParcel_native_readStringVector( size_t parentHandle; const string_vec *vec; - status_t err = parcel->readBuffer(&parentHandle, + status_t err = parcel->readBuffer(sizeof(*vec), &parentHandle, reinterpret_cast(&vec)); if (err != OK) { @@ -719,16 +719,15 @@ static jobjectArray JHwParcel_native_readStringVector( size_t childHandle; err = ::android::hardware::readEmbeddedFromParcel( - const_cast(vec), + const_cast(*vec), *parcel, parentHandle, 0 /* parentOffset */, &childHandle); for (size_t i = 0; (err == OK) && (i < vec->size()); ++i) { err = android::hardware::readEmbeddedFromParcel( - const_cast *>(vec), + const_cast((*vec)[i]), *parcel, childHandle, - i * sizeof(hidl_string), - nullptr /* childHandle */); + i * sizeof(hidl_string) /* parentOffset */); } if (err != OK) { @@ -810,13 +809,20 @@ static jobject JHwParcel_native_readStrongBinder(JNIEnv *env, jobject thiz) { return JHwRemoteBinder::NewObject(env, binder); } -static jobject JHwParcel_native_readBuffer(JNIEnv *env, jobject thiz) { +static jobject JHwParcel_native_readBuffer(JNIEnv *env, jobject thiz, + jlong expectedSize) { hardware::Parcel *parcel = JHwParcel::GetNativeContext(env, thiz)->getParcel(); size_t handle; const void *ptr; - status_t status = parcel->readBuffer(&handle, &ptr); + + if (expectedSize < 0) { + jniThrowException(env, "java/lang/IllegalArgumentException", NULL); + return nullptr; + } + + status_t status = parcel->readBuffer(expectedSize, &handle, &ptr); if (status != OK) { jniThrowException(env, "java/util/NoSuchElementException", NULL); @@ -827,8 +833,8 @@ static jobject JHwParcel_native_readBuffer(JNIEnv *env, jobject thiz) { } static jobject JHwParcel_native_readEmbeddedBuffer( - JNIEnv *env, jobject thiz, jlong parentHandle, jlong offset, - jboolean nullable) { + JNIEnv *env, jobject thiz, jlong expectedSize, + jlong parentHandle, jlong offset, jboolean nullable) { hardware::Parcel *parcel = JHwParcel::GetNativeContext(env, thiz)->getParcel(); @@ -836,8 +842,13 @@ static jobject JHwParcel_native_readEmbeddedBuffer( const void *ptr; status_t status = - parcel->readNullableEmbeddedBuffer(&childHandle, parentHandle, offset, - &ptr); + parcel->readNullableEmbeddedBuffer(expectedSize, + &childHandle, parentHandle, offset, &ptr); + + if (expectedSize < 0) { + jniThrowException(env, "java/lang/IllegalArgumentException", NULL); + return nullptr; + } if (status != OK) { jniThrowException(env, "java/util/NoSuchElementException", NULL); @@ -952,10 +963,10 @@ static JNINativeMethod gMethods[] = { { "send", "()V", (void *)JHwParcel_native_send }, - { "readBuffer", "()L" PACKAGE_PATH "/HwBlob;", + { "readBuffer", "(J)L" PACKAGE_PATH "/HwBlob;", (void *)JHwParcel_native_readBuffer }, - { "readEmbeddedBuffer", "(JJZ)L" PACKAGE_PATH "/HwBlob;", + { "readEmbeddedBuffer", "(JJJZ)L" PACKAGE_PATH "/HwBlob;", (void *)JHwParcel_native_readEmbeddedBuffer }, { "writeBuffer", "(L" PACKAGE_PATH "/HwBlob;)V",