Use offsets rather than pointers when patching android manifest

* Fixes a crash when using bytebuf now would cause resizes

renderdoc/android/android_manifest.cpp

@@ -30,15 +30,50 @@
 const uint32_t debuggableResourceId = 0x0101000f;
 const uint32_t addingStringIndex = 0x8b8b8b8b;
 
+// the string pool always immediately follows the XML header, which is just an empty header.
+const size_t stringpoolOffset = sizeof(ResChunk_header);
+
 namespace Android
 {
-rdcstr GetStringPoolValue(ResStringPool_header *stringpool, ResStringPool_ref ref)
+template <typename T>
+void SetFromBytes(bytebuf &bytes, size_t offs, const T &t)
 {
-  byte *base = (byte *)stringpool;
+  T *ptr = (T *)(bytes.data() + offs);
+  memcpy(ptr, &t, sizeof(T));
+}
 
-  uint32_t stringCount = stringpool->stringCount;
-  uint32_t *stringOffsets = (uint32_t *)(base + stringpool->header.headerSize);
-  byte *stringData = base + stringpool->stringsStart;
+template <typename T>
+T GetFromBytes(bytebuf &bytes, size_t offs)
+{
+  T ret;
+  T *ptr = (T *)(bytes.data() + offs);
+  memcpy(&ret, ptr, sizeof(T));
+  return ret;
+}
+
+template <typename T>
+void InsertBytes(bytebuf &bytes, size_t offs, const T &data)
+{
+  byte *byteData = (byte *)&data;
+
+  bytes.insert(offs, byteData, sizeof(T));
+}
+
+template <>
+void InsertBytes(bytebuf &bytes, size_t offs, const bytebuf &data)
+{
+  bytes.insert(offs, data);
+}
+
+rdcstr GetStringPoolValue(bytebuf &bytes, ResStringPool_ref ref)
+{
+  ResStringPool_header stringpool = GetFromBytes<ResStringPool_header>(bytes, stringpoolOffset);
+
+  byte *base = bytes.data() + stringpoolOffset;
+
+  uint32_t stringCount = stringpool.stringCount;
+  uint32_t *stringOffsets = (uint32_t *)(base + stringpool.header.headerSize);
+  byte *stringData = base + stringpool.stringsStart;
 
   if(ref.index == ~0U)
     return "";
@@ -49,7 +84,7 @@ rdcstr GetStringPoolValue(ResStringPool_header *stringpool, ResStringPool_ref re
   byte *strdata = stringData + stringOffsets[ref.index];
 
   // strdata now points at len characters of string. Check if it's UTF-8 or UTF-16
-  if((stringpool->flags & ResStringPool_header::UTF8_FLAG) == 0)
+  if((stringpool.flags & ResStringPool_header::UTF8_FLAG) == 0)
   {
     uint16_t *str = (uint16_t *)strdata;
 
@@ -114,127 +149,90 @@ void ShiftStringPoolValue(Res_value &val, uint32_t insertedLocation)
     val.data++;
 }
 
-template <typename T>
-void InsertBytes(bytebuf &bytes, byte *pos, const T &data)
-{
-  byte *byteData = (byte *)&data;
-
-  bytes.insert(pos - &bytes[0], byteData, sizeof(T));
-}
-
-template <>
-void InsertBytes(bytebuf &bytes, byte *pos, const bytebuf &data)
-{
-  bytes.insert(pos - &bytes[0], data);
-}
-
 bool PatchManifest(bytebuf &manifestBytes)
 {
-  // Whether to insert a new string & resource ID at the start or end of the resource map table. I
-  // can't find anything that indicates there is any required ordering to these, so either should be
-  // valid.
-  const bool insertStringAtStart = false;
-
-  // reserve room for our modifications up front, to be sure that if we do make them we'll never
-  // invalidate any pointers. We could add:
-  manifestBytes.reserve(
-      manifestBytes.size() +
-      // - a string (uint32 offset, uint16 length and string characters (possibly
-      //   in UTF-16) including NULL)
-      sizeof(uint32_t) + sizeof(uint16_t) + sizeof("debuggable") * 2 +
-      // - a resource ID mapping (one uint32)
-      sizeof(uint32_t) +
-      // - an attribute (ResXMLTree_attribute)
-      sizeof(ResXMLTree_attribute) +
-      // and we add 16 bytes more just for a safety margin with any necessary padding
-      16);
-
-  // save the capacity so we can check we never resize
-  size_t capacity = manifestBytes.capacity();
-
-  byte *start = &manifestBytes.front();
-
-  byte *cur = start;
-
-  ResChunk_header *xmlroot = (ResChunk_header *)cur;
-
-  if((byte *)(xmlroot + 1) > &manifestBytes.back())
+  if(manifestBytes.size() < sizeof(ResChunk_header))
   {
     RDCERR("Manifest is truncated, %zu bytes doesn't contain full XML header", manifestBytes.size());
     return false;
   }
 
-  if(xmlroot->type != ResType::XML)
+  size_t cur = 0;
+
+  ResChunk_header xmlroot = GetFromBytes<ResChunk_header>(manifestBytes, cur);
+
+  if(xmlroot.type != ResType::XML)
   {
-    RDCERR("XML Header is malformed, type is %u expected %u", xmlroot->type, ResType::XML);
+    RDCERR("XML Header is malformed, type is %u expected %u", xmlroot.type, ResType::XML);
     return false;
   }
 
-  if(xmlroot->headerSize != sizeof(*xmlroot))
+  if(xmlroot.headerSize != sizeof(xmlroot))
   {
     RDCERR("XML Header is malformed, header size is reported as %u but expected %u",
-           xmlroot->headerSize, sizeof(*xmlroot));
+           xmlroot.headerSize, sizeof(xmlroot));
     return false;
   }
 
   // this isn't necessarily fatal, but it is unexpected.
-  if(xmlroot->size != manifestBytes.size())
-    RDCWARN("XML header is malformed, size is reported as %u but %zu bytes found", xmlroot->size,
+  if(xmlroot.size != manifestBytes.size())
+    RDCWARN("XML header is malformed, size is reported as %u but %zu bytes found", xmlroot.size,
             manifestBytes.size());
 
-  cur += xmlroot->headerSize;
+  cur += xmlroot.headerSize;
 
-  ResStringPool_header *stringpool = (ResStringPool_header *)cur;
+  ResStringPool_header stringpool = GetFromBytes<ResStringPool_header>(manifestBytes, cur);
 
-  if(stringpool->header.type != ResType::StringPool)
+  if(stringpool.header.type != ResType::StringPool)
   {
-    RDCERR("Manifest format is unsupported, expected string pool but got %u",
-           stringpool->header.type);
+    RDCERR("Manifest format is unsupported, expected string pool but got %u", stringpool.header.type);
     return false;
   }
 
-  if(stringpool->header.headerSize != sizeof(*stringpool))
+  if(stringpool.header.headerSize != sizeof(stringpool))
   {
     RDCERR("String pool is malformed, header size is reported as %u but expected %u",
-           stringpool->header.headerSize, sizeof(*stringpool));
+           stringpool.header.headerSize, sizeof(stringpool));
     return false;
   }
 
-  if(cur + stringpool->header.size > &manifestBytes.back())
+  if(cur + stringpool.header.size > manifestBytes.size())
   {
     RDCERR("String pool is truncated, expected %u more bytes but only have %u",
-           stringpool->header.size, uint32_t(&manifestBytes.back() - cur));
+           stringpool.header.size, manifestBytes.size() - cur);
     return false;
   }
 
-  cur += stringpool->header.size;
+  cur += stringpool.header.size;
 
-  ResChunk_header *resMap = (ResChunk_header *)cur;
+  ResChunk_header resMap = GetFromBytes<ResChunk_header>(manifestBytes, cur);
+  const size_t resMapOffset = cur;
 
-  if(resMap->type != ResType::ResourceMap)
+  if(resMap.type != ResType::ResourceMap)
   {
-    RDCERR("Manifest format is unsupported, expected resource table but got %u", resMap->type);
+    RDCERR("Manifest format is unsupported, expected resource table but got %u", resMap.type);
     return false;
   }
 
-  if(resMap->headerSize != sizeof(*resMap))
+  if(resMap.headerSize != sizeof(resMap))
   {
     RDCERR("Resource map is malformed, header size is reported as %u but expected %u",
-           resMap->headerSize, sizeof(*resMap));
+           resMap.headerSize, sizeof(resMap));
     return false;
   }
 
-  if(cur + resMap->size > &manifestBytes.back())
+  if(cur + resMap.size > manifestBytes.size())
   {
-    RDCERR("Resource map is truncated, expected %u more bytes but only have %u", resMap->size,
-           uint32_t(&manifestBytes.back() - cur));
+    RDCERR("Resource map is truncated, expected %u more bytes but only have %u", resMap.size,
+           manifestBytes.size() - cur);
     return false;
   }
 
-  uint32_t *resourceMapping = (uint32_t *)(cur + resMap->headerSize);
-  uint32_t resourceMappingCount = (resMap->size - resMap->headerSize) / sizeof(uint32_t);
+  const uint32_t resourceMappingCount = (resMap.size - resMap.headerSize) / sizeof(uint32_t);
+  const rdcarray<uint32_t> resourceMapping(
+      (const uint32_t *)(manifestBytes.data() + cur + resMap.headerSize), resourceMappingCount);
 
-  cur += resMap->size;
+  cur += resMap.size;
 
   bool stringAdded = false;
 
@@ -242,50 +240,52 @@ bool PatchManifest(bytebuf &manifestBytes)
   // close. Since the <application> tag is only valid in one place in the XML we can just continue
   // iterating until we find it - we don't actually need to care about the structure of the XML
   // since we are identifying a unique tag and adding one attribute.
-  while(cur < &manifestBytes.back())
+  while(cur < manifestBytes.size())
   {
-    ResChunk_header *node = (ResChunk_header *)cur;
+    ResChunk_header node = GetFromBytes<ResChunk_header>(manifestBytes, cur);
 
-    if(node->type != ResType::StartElement)
+    if(node.type != ResType::StartElement)
     {
-      cur += node->size;
+      cur += node.size;
       continue;
     }
 
-    ResXMLTree_attrExt *startElement = (ResXMLTree_attrExt *)(cur + node->headerSize);
+    ResXMLTree_attrExt startElement =
+        GetFromBytes<ResXMLTree_attrExt>(manifestBytes, cur + node.headerSize);
 
-    rdcstr name = GetStringPoolValue(stringpool, startElement->name);
+    rdcstr name = GetStringPoolValue(manifestBytes, startElement.name);
 
     if(name != "application")
     {
-      cur += node->size;
+      cur += node.size;
       continue;
     }
 
     // found the application tag! Now search its attribtues to see if it already has a debuggable
     // attribute (that might be set explicitly to false instead of defaulting)
-    if(startElement->attributeSize != sizeof(ResXMLTree_attribute))
+    if(startElement.attributeSize != sizeof(ResXMLTree_attribute))
     {
       RDCWARN("Declared attribute size %u doesn't match what we expect %zu",
-              startElement->attributeSize, sizeof(ResXMLTree_attribute));
+              startElement.attributeSize, sizeof(ResXMLTree_attribute));
     }
 
-    if(startElement->attributeStart != sizeof(*startElement))
+    if(startElement.attributeStart != sizeof(startElement))
     {
       RDCWARN("Declared attribute start offset %u doesn't match what we expect %zu",
-              startElement->attributeStart, sizeof(*startElement));
+              startElement.attributeStart, sizeof(startElement));
     }
 
-    byte *attributesStart = cur + node->headerSize + startElement->attributeStart;
+    const size_t attributeStartOffset = cur + node.headerSize + startElement.attributeStart;
+    byte *attributesStart = manifestBytes.data() + attributeStartOffset;
 
     bool found = false;
 
-    for(uint32_t i = 0; i < startElement->attributeCount; i++)
+    for(uint32_t i = 0; i < startElement.attributeCount; i++)
     {
       ResXMLTree_attribute *attribute =
-          (ResXMLTree_attribute *)(attributesStart + startElement->attributeSize * i);
+          (ResXMLTree_attribute *)(attributesStart + startElement.attributeSize * i);
 
-      rdcstr attr = GetStringPoolValue(stringpool, attribute->name);
+      rdcstr attr = GetStringPoolValue(manifestBytes, attribute->name);
 
       if(attr != "debuggable")
         continue;
@@ -313,7 +313,7 @@ bool PatchManifest(bytebuf &manifestBytes)
           if(attribute->rawValue.index != ~0U)
           {
             RDCWARN("attribute has raw value '%s' which we aren't patching",
-                    GetStringPoolValue(stringpool, attribute->rawValue).c_str());
+                    GetStringPoolValue(manifestBytes, attribute->rawValue).c_str());
           }
 
           // we'll still add a debuggable attribute that is resource ID linked, so we don't mark the
@@ -345,7 +345,7 @@ bool PatchManifest(bytebuf &manifestBytes)
         if(attribute->rawValue.index != ~0U)
         {
           RDCWARN("attribute has raw value '%s' which we aren't patching",
-                  GetStringPoolValue(stringpool, attribute->rawValue).c_str());
+                  GetStringPoolValue(manifestBytes, attribute->rawValue).c_str());
         }
       }
 
@@ -356,10 +356,9 @@ bool PatchManifest(bytebuf &manifestBytes)
     if(found)
       break;
 
-    if(startElement->attributeSize != sizeof(ResXMLTree_attribute))
+    if(startElement.attributeSize != sizeof(ResXMLTree_attribute))
     {
-      RDCERR("Unexpected attribute size %u, can't add missing attribute",
-             startElement->attributeSize);
+      RDCERR("Unexpected attribute size %u, can't add missing attribute", startElement.attributeSize);
       return false;
     }
 
@@ -375,7 +374,7 @@ bool PatchManifest(bytebuf &manifestBytes)
     {
       if(resourceMapping[i] == debuggableResourceId)
       {
-        rdcstr str = GetStringPoolValue(stringpool, {i});
+        rdcstr str = GetStringPoolValue(manifestBytes, {i});
 
         if(str != "debuggable")
         {
@@ -399,9 +398,9 @@ bool PatchManifest(bytebuf &manifestBytes)
     debuggable.typedValue.data = ~0U;
 
     // search the stringpool for the schema, it should be there already.
-    for(uint32_t i = 0; i < stringpool->stringCount; i++)
+    for(uint32_t i = 0; i < stringpool.stringCount; i++)
     {
-      rdcstr val = GetStringPoolValue(stringpool, {i});
+      rdcstr val = GetStringPoolValue(manifestBytes, {i});
       if(val == "http://schemas.android.com/apk/res/android")
       {
         debuggable.ns.index = i;
@@ -419,16 +418,16 @@ bool PatchManifest(bytebuf &manifestBytes)
     // ID than ours (in which case we're inserting it in the right place).
     uint32_t attributeInsertIndex = 0;
 
-    for(uint32_t i = 0; i < startElement->attributeCount; i++)
+    for(uint32_t i = 0; i < startElement.attributeCount; i++)
     {
       ResXMLTree_attribute *attr =
-          (ResXMLTree_attribute *)(attributesStart + startElement->attributeSize * i);
+          (ResXMLTree_attribute *)(attributesStart + startElement.attributeSize * i);
 
       if(attr->name.index >= resourceMappingCount)
       {
         attributeInsertIndex = i;
         RDCDEBUG("Inserting attribute before %s, with no resource ID",
-                 GetStringPoolValue(stringpool, attr->name).c_str());
+                 GetStringPoolValue(manifestBytes, attr->name).c_str());
         break;
       }
 
@@ -438,20 +437,23 @@ bool PatchManifest(bytebuf &manifestBytes)
       {
         attributeInsertIndex = i;
         RDCDEBUG("Inserting attribute before %s, with resource ID %x",
-                 GetStringPoolValue(stringpool, attr->name).c_str(), resourceId);
+                 GetStringPoolValue(manifestBytes, attr->name).c_str(), resourceId);
         break;
       }
 
       RDCDEBUG("Skipping past attribute %s, with resource ID %x",
-               GetStringPoolValue(stringpool, attr->name).c_str(), resourceId);
+               GetStringPoolValue(manifestBytes, attr->name).c_str(), resourceId);
     }
 
-    InsertBytes(manifestBytes, attributesStart + startElement->attributeSize * attributeInsertIndex,
-                debuggable);
+    InsertBytes(manifestBytes,
+                attributeStartOffset + startElement.attributeSize * attributeInsertIndex, debuggable);
 
     // update header
-    startElement->attributeCount++;
-    node->size += sizeof(ResXMLTree_attribute);
+    node.size += sizeof(ResXMLTree_attribute);
+    SetFromBytes(manifestBytes, cur, node);
+
+    startElement.attributeCount++;
+    SetFromBytes(manifestBytes, cur + node.headerSize, startElement);
 
     stringAdded = (stringIndex.index == addingStringIndex);
 
@@ -463,95 +465,96 @@ bool PatchManifest(bytebuf &manifestBytes)
   // with insertions
   if(stringAdded)
   {
-    uint32_t insertIdx = insertStringAtStart ? 0 : resourceMappingCount;
+    uint32_t insertIdx = resourceMappingCount;
 
-    // add to the resource map
+    // add to the resource map first because it's after the string pool, that way we don't have to
+    // account for string pool modifications in resMapOffset
     {
-      if(insertIdx == 0)
-        InsertBytes(manifestBytes, (byte *)resMap + resMap->headerSize, debuggableResourceId);
-      else
-        InsertBytes(manifestBytes, (byte *)resMap + resMap->size, debuggableResourceId);
-      resMap->size += sizeof(uint32_t);
+      InsertBytes(manifestBytes, resMapOffset + resMap.size, debuggableResourceId);
+      resMap.size += sizeof(uint32_t);
+      SetFromBytes(manifestBytes, resMapOffset, resMap);
     }
 
-    // add to the string pool
+    // add to the string pool next
     {
       // add the offset
-      stringpool->header.size += sizeof(uint32_t);
-      stringpool->stringCount++;
-      stringpool->stringsStart += sizeof(uint32_t);
+      stringpool.header.size += sizeof(uint32_t);
+      stringpool.stringCount++;
+      stringpool.stringsStart += sizeof(uint32_t);
       // if we're adding a string we don't bother to do it sorted, so remove the sorted flag
-      stringpool->flags =
-          ResStringPool_header::StringFlags(stringpool->flags & ~ResStringPool_header::SORTED_FLAG);
+      stringpool.flags =
+          ResStringPool_header::StringFlags(stringpool.flags & ~ResStringPool_header::SORTED_FLAG);
 
-      byte *base = (byte *)stringpool;
+      size_t stringpoolStringOffsetsOffset = stringpoolOffset + stringpool.header.headerSize;
 
-      uint32_t *stringOffsets = (uint32_t *)(base + stringpool->header.headerSize);
+      // we insert a zero offset at the position we're inserting. Then when we fix up that and all
+      // subsequent offsets
+      InsertBytes(manifestBytes, stringpoolStringOffsetsOffset + sizeof(uint32_t) * insertIdx,
+                  uint32_t(0));
 
-      // we duplicate the offset at the position we're inserting. Then when we fix up all the other
-      // offsets the duplicated one shifts by the right amount.
-      InsertBytes(manifestBytes,
-                  (byte *)stringpool + stringpool->header.headerSize + sizeof(uint32_t) * insertIdx,
-                  stringOffsets[insertIdx]);
+      bytebuf stringbytes;
 
-      uint32_t shift = 0;
-
-      byte *stringData = (byte *)stringpool + stringpool->stringsStart;
-
-      // insert the string, with length prefix and trailing NULL
-      if(stringpool->flags & ResStringPool_header::UTF8_FLAG)
+      // construct the string, with length prefix and trailing NULL
+      if(stringpool.flags & ResStringPool_header::UTF8_FLAG)
       {
-        bytebuf bytes = {0xA, 0xA, 'd', 'e', 'b', 'u', 'g', 'g', 'a', 'b', 'l', 'e', 0};
-        shift = (uint32_t)bytes.size();
-
-        InsertBytes(manifestBytes, stringData + stringOffsets[insertIdx], bytes);
+        stringbytes = {0xA, 0xA, 'd', 'e', 'b', 'u', 'g', 'g', 'a', 'b', 'l', 'e', 0};
       }
       else
       {
-        bytebuf bytes = {0xA, 0x0, 'd', 0, 'e', 0, 'b', 0, 'u', 0, 'g', 0,
-                         'g', 0,   'a', 0, 'b', 0, 'l', 0, 'e', 0, 0,   0};
-        shift = (uint32_t)bytes.size();
-
-        InsertBytes(manifestBytes, stringData + stringOffsets[insertIdx], bytes);
+        stringbytes = {0xA, 0x0, 'd', 0, 'e', 0, 'b', 0, 'u', 0, 'g', 0,
+                       'g', 0,   'a', 0, 'b', 0, 'l', 0, 'e', 0, 0,   0};
       }
 
       // account for added string
-      stringpool->header.size += shift;
+      stringpool.header.size += stringbytes.count();
 
       // shift all the offsets *after* the string we inserted (we inserted precisely at that
       // offset).
-      for(uint32_t i = insertIdx + 1; i < stringpool->stringCount; i++)
-        stringOffsets[i] += shift;
+      uint32_t *stringOffsets = (uint32_t *)(manifestBytes.data() + stringpoolStringOffsetsOffset);
+
+      // the one we inserted will be inserted at the offset of whichever was previously at that
+      // index (which is now one further on)
+      stringOffsets[insertIdx] = stringOffsets[insertIdx + 1];
+
+      for(uint32_t i = insertIdx + 1; i < stringpool.stringCount; i++)
+        stringOffsets[i] += stringbytes.count();
+
+      // now insert the string bytes
+      InsertBytes(manifestBytes,
+                  stringpoolOffset + stringpool.stringsStart + stringOffsets[insertIdx], stringbytes);
 
       // if the stringpool isn't integer aligned, add padding bytes
-      uint32_t alignedSize = AlignUp4(stringpool->header.size);
+      uint32_t alignedSize = AlignUp4(stringpool.header.size);
 
-      if(alignedSize > stringpool->header.size)
+      if(alignedSize > stringpool.header.size)
       {
-        uint32_t paddingLen = alignedSize - stringpool->header.size;
+        uint32_t paddingLen = alignedSize - stringpool.header.size;
 
         RDCDEBUG("Inserting %u padding bytes to align %u up to %u", paddingLen,
-                 stringpool->header.size, alignedSize);
+                 stringpool.header.size, alignedSize);
 
         bytebuf padding;
         padding.resize(paddingLen);
 
-        InsertBytes(manifestBytes, base + stringpool->header.size, padding);
+        InsertBytes(manifestBytes, stringpoolOffset + stringpool.header.size, padding);
 
-        stringpool->header.size += paddingLen;
+        stringpool.header.size += paddingLen;
       }
+
+      // write the updated stringpool
+      SetFromBytes(manifestBytes, stringpoolOffset, stringpool);
     }
 
     // now iterate over all nodes and fixup any stringrefs pointing after our insert point
-    cur = start + xmlroot->headerSize;
-    // skip string pool
-    cur += ((ResChunk_header *)cur)->size;
-    // skip resource map
-    cur += ((ResChunk_header *)cur)->size;
+    byte *ptr = manifestBytes.data() + xmlroot.headerSize;
+    // skip string pool, whatever size it is now
+    ptr += ((ResChunk_header *)ptr)->size;
+    // skip resource map, whatever size it is now
+    ptr += ((ResChunk_header *)ptr)->size;
 
-    while(cur < &manifestBytes.back())
+    while(ptr < manifestBytes.end())
     {
-      ResXMLTree_node *node = (ResXMLTree_node *)cur;
+      ResXMLTree_node *node = (ResXMLTree_node *)ptr;
 
       if(node->header.headerSize != sizeof(*node))
         RDCWARN("Headersize was reported as %u, but we expected ResXMLTree_node size %zu",
@@ -565,7 +568,7 @@ bool PatchManifest(bytebuf &manifestBytes)
         case ResType::NamespaceStart:
         case ResType::NamespaceEnd:
         {
-          ResXMLTree_namespaceExt *ns = (ResXMLTree_namespaceExt *)(cur + node->header.headerSize);
+          ResXMLTree_namespaceExt *ns = (ResXMLTree_namespaceExt *)(ptr + node->header.headerSize);
 
           ShiftStringPoolValue(ns->prefix, insertIdx);
           ShiftStringPoolValue(ns->uri, insertIdx);
@@ -574,7 +577,7 @@ bool PatchManifest(bytebuf &manifestBytes)
         case ResType::EndElement:
         {
           ResXMLTree_endElementExt *endElement =
-              (ResXMLTree_endElementExt *)(cur + node->header.headerSize);
+              (ResXMLTree_endElementExt *)(ptr + node->header.headerSize);
 
           ShiftStringPoolValue(endElement->ns, insertIdx);
           ShiftStringPoolValue(endElement->name, insertIdx);
@@ -582,7 +585,7 @@ bool PatchManifest(bytebuf &manifestBytes)
         }
         case ResType::CDATA:
         {
-          ResXMLTree_cdataExt *cdata = (ResXMLTree_cdataExt *)(cur + node->header.headerSize);
+          ResXMLTree_cdataExt *cdata = (ResXMLTree_cdataExt *)(ptr + node->header.headerSize);
 
           ShiftStringPoolValue(cdata->data, insertIdx);
           ShiftStringPoolValue(cdata->typedData, insertIdx);
@@ -590,13 +593,13 @@ bool PatchManifest(bytebuf &manifestBytes)
         }
         case ResType::StartElement:
         {
-          ResXMLTree_attrExt *startElement = (ResXMLTree_attrExt *)(cur + node->header.headerSize);
+          ResXMLTree_attrExt *startElement = (ResXMLTree_attrExt *)(ptr + node->header.headerSize);
 
           ShiftStringPoolValue(startElement->ns, insertIdx);
           ShiftStringPoolValue(startElement->name, insertIdx);
 
           // update attributes
-          byte *attributesStart = cur + node->header.headerSize + startElement->attributeStart;
+          byte *attributesStart = ptr + node->header.headerSize + startElement->attributeStart;
 
           for(uint32_t i = 0; i < startElement->attributeCount; i++)
           {
@@ -615,18 +618,12 @@ bool PatchManifest(bytebuf &manifestBytes)
           return false;
       }
 
-      cur += node->header.size;
+      ptr += node->header.size;
     }
   }
 
-  xmlroot->size = (uint32_t)manifestBytes.size();
-
-  if(manifestBytes.capacity() > capacity)
-  {
-    RDCERR(
-        "manifest vector resized during patching! Update reserve() at the start of "
-        "Android::PatchManifest");
-  }
+  xmlroot.size = (uint32_t)manifestBytes.size();
+  SetFromBytes(manifestBytes, 0, xmlroot);
 
   return true;
 }