Handle malformed manifests in printXMLBlock

Specially crafted manifest files can cause a segfault in printXMLBlock()
using improper tag nesting (without evaluating to
ResXMLTree::BAD_DOCUMENT). This fix checks and breaks when this
condition is detected.

Bug: 15549617
Change-Id: I27997fda86d228e993217a0c09993bff404cf317

1 files changed, 22 insertions, 14 deletions

diff --git a/tools/aapt/XMLNode.cpp b/tools/aapt/XMLNode.cpp
index b38b2ed..d2cd2d6 100644
--- a/tools/aapt/XMLNode.cpp
+++ b/tools/aapt/XMLNode.cpp
@@ -466,7 +466,7 @@ void printXMLBlock(ResXMLTree* block)
     block->restart();
 
     Vector<namespace_entry> namespaces;
-    
+
     ResXMLTree::event_code_t code;
     int depth = 0;
     while ((code=block->next()) != ResXMLTree::END_DOCUMENT && code != ResXMLTree::BAD_DOCUMENT) {
@@ -520,7 +520,12 @@ void printXMLBlock(ResXMLTree* block)
                 printf("\n");
             }
         } else if (code == ResXMLTree::END_TAG) {
-            depth--;
+            // Invalid tag nesting can be misused to break the parsing
+            // code below. Break if detected.
+            if (--depth < 0) {
+                printf("***BAD DEPTH in XMLBlock: %d\n", depth);
+                break;
+            }
         } else if (code == ResXMLTree::START_NAMESPACE) {
             namespace_entry ns;
             size_t len;
@@ -536,7 +541,10 @@ void printXMLBlock(ResXMLTree* block)
                     ns.uri.string());
             depth++;
         } else if (code == ResXMLTree::END_NAMESPACE) {
-            depth--;
+            if (--depth < 0) {
+                printf("***BAD DEPTH in XMLBlock: %d\n", depth);
+                break;
+            }
             const namespace_entry& ns = namespaces.top();
             size_t len;
             const char16_t* prefix16 = block->getNamespacePrefix(&len);
@@ -714,7 +722,7 @@ const String8& XMLNode::getFilename() const
 {
     return mFilename;
 }
-    
+
 const Vector<XMLNode::attribute_entry>&
     XMLNode::getAttributes() const
 {
@@ -730,7 +738,7 @@ const XMLNode::attribute_entry* XMLNode::getAttribute(const String16& ns,
             return &ae;
         }
     }
-    
+
     return NULL;
 }
 
@@ -774,14 +782,14 @@ sp<XMLNode> XMLNode::searchElement(const String16& tagNamespace, const String16&
             && mElementName == tagName) {
         return this;
     }
-    
+
     for (size_t i=0; i<mChildren.size(); i++) {
         sp<XMLNode> found = mChildren.itemAt(i)->searchElement(tagNamespace, tagName);
         if (found != NULL) {
             return found;
         }
     }
-    
+
     return NULL;
 }
 
@@ -795,7 +803,7 @@ sp<XMLNode> XMLNode::getChildElement(const String16& tagNamespace, const String1
             return child;
         }
     }
-    
+
     return NULL;
 }
 
@@ -977,7 +985,7 @@ status_t XMLNode::parseValues(const sp<AaptAssets>& assets,
                               ResourceTable* table)
 {
     bool hasErrors = false;
-    
+
     if (getType() == TYPE_ELEMENT) {
         const size_t N = mAttributes.size();
         String16 defPackage(assets->getPackage());
@@ -1013,7 +1021,7 @@ status_t XMLNode::assignResourceIds(const sp<AaptAssets>& assets,
                                     const ResourceTable* table)
 {
     bool hasErrors = false;
-    
+
     if (getType() == TYPE_ELEMENT) {
         String16 attr("attr");
         const char* errorMsg;
@@ -1093,7 +1101,7 @@ status_t XMLNode::flatten(const sp<AaptFile>& dest,
 {
     StringPool strings(mUTF8);
     Vector<uint32_t> resids;
-    
+
     // First collect just the strings for attribute names that have a
     // resource ID assigned to them.  This ensures that the resource ID
     // array is compact, and makes it easier to deal with attribute names
@@ -1141,7 +1149,7 @@ status_t XMLNode::flatten(const sp<AaptFile>& dest,
                 dest->getSize(), (stringPool->getSize()*100)/dest->getSize(),
                 dest->getPath().string());
     }
-        
+
     return NO_ERROR;
 }
 
@@ -1217,7 +1225,7 @@ XMLNode::startNamespace(void *userData, const char *prefix, const char *uri)
         printf("Start Namespace: %s %s\n", prefix, uri);
     }
     ParseState* st = (ParseState*)userData;
-    sp<XMLNode> node = XMLNode::newNamespace(st->filename, 
+    sp<XMLNode> node = XMLNode::newNamespace(st->filename,
             String16(prefix != NULL ? prefix : ""), String16(uri));
     node->setStartLineNumber(XML_GetCurrentLineNumber(st->parser));
     if (st->stack.size() > 0) {
@@ -1338,7 +1346,7 @@ status_t XMLNode::collect_strings(StringPool* dest, Vector<uint32_t>* outResIds,
         bool stripComments, bool stripRawValues) const
 {
     collect_attr_strings(dest, outResIds, true);
-    
+
     int i;
     if (RESOURCES_TOOLS_NAMESPACE != mNamespaceUri) {
         if (mNamespacePrefix.size() > 0) {