Validate incoming data properly.

readEmbeddedBuffer() now requires the correct length
of the buffer as an argument.

Modified the readEmbeddedFromParcel() functions to take
a const-reference to the struct the data is embedded in;
the kernel has already fixed up the pointers, we just need
to verify the buffer has the correct size, parent buffer
and offset within that parent buffer.

Bug: 30498700
Test: hidl_test, hidl_test_java, YouTube, Maps, Netflix, Camera
Change-Id: I44d6ca7ef6f252f8154b03ff9914b7db69c70604

2 files changed, 24 insertions, 9 deletions

diff --git a/transport/HidlBinderSupport.cpp b/transport/HidlBinderSupport.cpp
index c0601ca..f421953 100644
--- a/transport/HidlBinderSupport.cpp
+++ b/transport/HidlBinderSupport.cpp
@@ -33,7 +33,7 @@ const size_t hidl_memory::kOffsetOfName = offsetof(hidl_memory, mName);
 static_assert(hidl_memory::kOffsetOfHandle == 0, "wrong offset");
 static_assert(hidl_memory::kOffsetOfName == 24, "wrong offset");
 
-status_t readEmbeddedFromParcel(hidl_memory * /* memory */,
+status_t readEmbeddedFromParcel(const hidl_memory& memory,
         const Parcel &parcel, size_t parentHandle, size_t parentOffset) {
     const native_handle_t *handle;
     ::android::status_t _hidl_err = parcel.readNullableEmbeddedNativeHandle(
@@ -43,7 +43,7 @@ status_t readEmbeddedFromParcel(hidl_memory * /* memory */,
 
     if (_hidl_err == ::android::OK) {
         _hidl_err = readEmbeddedFromParcel(
-                (hidl_string*) nullptr,
+                memory.name(),
                 parcel,
                 parentHandle,
                 parentOffset + hidl_memory::kOffsetOfName);
@@ -73,14 +73,28 @@ status_t writeEmbeddedToParcel(const hidl_memory &memory,
 const size_t hidl_string::kOffsetOfBuffer = offsetof(hidl_string, mBuffer);
 static_assert(hidl_string::kOffsetOfBuffer == 0, "wrong offset");
 
-status_t readEmbeddedFromParcel(hidl_string * /* string */,
+status_t readEmbeddedFromParcel(const hidl_string &string ,
         const Parcel &parcel, size_t parentHandle, size_t parentOffset) {
     const void *out;
-    return parcel.readEmbeddedBuffer(
+
+    status_t status = parcel.readEmbeddedBuffer(
+            string.size() + 1,
             nullptr /* buffer_handle */,
             parentHandle,
             parentOffset + hidl_string::kOffsetOfBuffer,
             &out);
+
+    if (status != OK) {
+        return status;
+    }
+
+    // Always safe to access out[string.size()] because we read size+1 bytes
+    if (static_cast<const char *>(out)[string.size()] != '\0') {
+        ALOGE("Received unterminated hidl_string buffer.");
+        return BAD_VALUE;
+    }
+
+    return OK;
 }
 
 status_t writeEmbeddedToParcel(const hidl_string &string,
diff --git a/transport/include/hidl/HidlBinderSupport.h b/transport/include/hidl/HidlBinderSupport.h
index 86ac74c..302e289 100644
--- a/transport/include/hidl/HidlBinderSupport.h
+++ b/transport/include/hidl/HidlBinderSupport.h
@@ -60,7 +60,7 @@ private:
 
 // ---------------------- hidl_memory
 
-status_t readEmbeddedFromParcel(hidl_memory *memory,
+status_t readEmbeddedFromParcel(const hidl_memory &memory,
         const Parcel &parcel, size_t parentHandle, size_t parentOffset);
 
 status_t writeEmbeddedToParcel(const hidl_memory &memory,
@@ -68,7 +68,7 @@ status_t writeEmbeddedToParcel(const hidl_memory &memory,
 
 // ---------------------- hidl_string
 
-status_t readEmbeddedFromParcel(hidl_string *string,
+status_t readEmbeddedFromParcel(const hidl_string &string,
         const Parcel &parcel, size_t parentHandle, size_t parentOffset);
 
 status_t writeEmbeddedToParcel(const hidl_string &string,
@@ -92,13 +92,14 @@ status_t writeToParcel(const Status &status, Parcel* parcel);
 
 template<typename T>
 status_t readEmbeddedFromParcel(
-        hidl_vec<T> * /*vec*/,
+        const hidl_vec<T> &vec,
         const Parcel &parcel,
         size_t parentHandle,
         size_t parentOffset,
         size_t *handle) {
     const void *out;
     return parcel.readNullableEmbeddedBuffer(
+            vec.size() * sizeof(T),
             handle,
             parentHandle,
             parentOffset + hidl_vec<T>::kOffsetOfBuffer,
@@ -129,7 +130,7 @@ status_t findInParcel(const hidl_vec<T> &vec, const Parcel &parcel, size_t *hand
 
 template<typename T, MQFlavor flavor>
 ::android::status_t readEmbeddedFromParcel(
-        MQDescriptor<T, flavor> *obj,
+        MQDescriptor<T, flavor> &obj,
         const ::android::hardware::Parcel &parcel,
         size_t parentHandle,
         size_t parentOffset) {
@@ -138,7 +139,7 @@ template<typename T, MQFlavor flavor>
     size_t _hidl_grantors_child;
 
     _hidl_err = ::android::hardware::readEmbeddedFromParcel(
-                &obj->grantors(),
+                obj.grantors(),
                 parcel,
                 parentHandle,
                 parentOffset + MQDescriptor<T, flavor>::kOffsetOfGrantors,