stagefright: fix AMessage::FromParcel

Add check for incoming mNumItems. Also add check readCString return
value.

Fix style & add log.

Bug: 24123723

Change-Id: If41a5312c27d868f481893eef56019b6807c39b7

1 files changed, 25 insertions, 3 deletions

diff --git a/media/libstagefright/foundation/AMessage.cpp b/media/libstagefright/foundation/AMessage.cpp
index 795e8a6..26bea2c 100644
--- a/media/libstagefright/foundation/AMessage.cpp
+++ b/media/libstagefright/foundation/AMessage.cpp
@@ -535,13 +535,24 @@ sp<AMessage> AMessage::FromParcel(const Parcel &parcel) {
     sp<AMessage> msg = new AMessage(what);
 
     msg->mNumItems = static_cast<size_t>(parcel.readInt32());
+    if (msg->mNumItems > kMaxNumItems) {
+        ALOGE("Too large number of items clipped.");
+        msg->mNumItems = kMaxNumItems;
+    }
+
     for (size_t i = 0; i < msg->mNumItems; ++i) {
         Item *item = &msg->mItems[i];
 
         const char *name = parcel.readCString();
-        item->setName(name, strlen(name));
-        item->mType = static_cast<Type>(parcel.readInt32());
+        if (name == NULL) {
+            ALOGE("Failed reading name for an item. Parsing aborted.");
+            msg->mNumItems = i;
+            break;
+        }
 
+        item->mType = static_cast<Type>(parcel.readInt32());
+        // setName() happens below so that we don't leak memory when parsing
+        // is aborted in the middle.
         switch (item->mType) {
             case kTypeInt32:
             {
@@ -575,7 +586,16 @@ sp<AMessage> AMessage::FromParcel(const Parcel &parcel) {
 
             case kTypeString:
             {
-                item->u.stringValue = new AString(parcel.readCString());
+                const char *stringValue = parcel.readCString();
+                if (stringValue == NULL) {
+                    ALOGE("Failed reading string value from a parcel. "
+                        "Parsing aborted.");
+                    msg->mNumItems = i;
+                    continue;
+                    // The loop will terminate subsequently.
+                } else {
+                    item->u.stringValue = new AString(stringValue);
+                }
                 break;
             }
 
@@ -594,6 +614,8 @@ sp<AMessage> AMessage::FromParcel(const Parcel &parcel) {
                 TRESPASS();
             }
         }
+
+        item->setName(name, strlen(name));
     }
 
     return msg;