From ddd346c7d54519e056b5b8b6d58b647770b3bb01 Mon Sep 17 00:00:00 2001
From: Flanker <i@flanker017.me>
Date: Fri, 11 Sep 2015 19:05:47 +0800
Subject: stagefright: fix AMessage::FromParcel

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

Fix style & add log.

Bug: 24123723

Change-Id: If41a5312c27d868f481893eef56019b6807c39b7
---
 media/libstagefright/foundation/AMessage.cpp | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

(limited to 'media/libstagefright/foundation')

diff --git a/media/libstagefright/foundation/AMessage.cpp b/media/libstagefright/foundation/AMessage.cpp
index e549ff6..725a574 100644
--- a/media/libstagefright/foundation/AMessage.cpp
+++ b/media/libstagefright/foundation/AMessage.cpp
@@ -601,13 +601,24 @@ sp<AMessage> AMessage::FromParcel(const Parcel &parcel) {
     msg->setWhat(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:
             {
@@ -641,7 +652,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;
             }
 
@@ -660,6 +680,8 @@ sp<AMessage> AMessage::FromParcel(const Parcel &parcel) {
                 TRESPASS();
             }
         }
+
+        item->setName(name, strlen(name));
     }
 
     return msg;
-- 
cgit v1.1