Bug 485941 - Stack overflow using overly-deep XML tree (DoS). r=bzbarsky
authorPeter Van der Beken <peterv@propagandism.org>
Fri, 13 Dec 2019 17:53:41 +0000
changeset 507532 8986a5aaa6b83e99c8c19b29adc09594269936fb
parent 507531 2c0887a202ad43f304a5f8db54876a0798ba316a
child 507533 e49dc46498238e04c0571a06f3e95574e1a0ddf5
push id36930
push userncsoregi@mozilla.com
push dateWed, 18 Dec 2019 21:12:21 +0000
treeherdermozilla-central@f870bccd07ee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs485941
milestone73.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 485941 - Stack overflow using overly-deep XML tree (DoS). r=bzbarsky Differential Revision: https://phabricator.services.mozilla.com/D56883
parser/htmlparser/nsExpatDriver.cpp
parser/htmlparser/nsExpatDriver.h
testing/web-platform/mozilla/tests/xml/parsedepth.html
--- a/parser/htmlparser/nsExpatDriver.cpp
+++ b/parser/htmlparser/nsExpatDriver.cpp
@@ -21,31 +21,36 @@
 #include "nsIScriptError.h"
 #include "nsIContentPolicy.h"
 #include "nsContentPolicyUtils.h"
 #include "nsError.h"
 #include "nsXPCOMCIDInternal.h"
 #include "nsUnicharInputStream.h"
 #include "nsContentUtils.h"
 #include "mozilla/BasePrincipal.h"
+#include "mozilla/IntegerTypeTraits.h"
 #include "mozilla/NullPrincipal.h"
 
 #include "mozilla/Logging.h"
 
 using mozilla::fallible;
 using mozilla::LogLevel;
 using mozilla::MakeStringSpan;
 using mozilla::dom::Document;
 
 #define kExpatSeparatorChar 0xFFFF
 
 static const char16_t kUTF16[] = {'U', 'T', 'F', '-', '1', '6', '\0'};
 
 static mozilla::LazyLogModule gExpatDriverLog("expatdriver");
 
+// Use the same maximum tree depth as Chromium (see
+// https://chromium.googlesource.com/chromium/src/+/f464165c1dedff1c955d3c051c5a9a1c6a0e8f6b/third_party/WebKit/Source/core/xml/parser/XMLDocumentParser.cpp#85).
+static const uint16_t sMaxXMLTreeDepth = 5000;
+
 /***************************** EXPAT CALL BACKS ******************************/
 // The callback handlers that get called from the expat parser.
 
 static void Driver_HandleXMLDeclaration(void* aUserData,
                                         const XML_Char* aVersion,
                                         const XML_Char* aEncoding,
                                         int aStandalone) {
   NS_ASSERTION(aUserData, "expat driver should exist");
@@ -241,16 +246,17 @@ nsExpatDriver::nsExpatDriver()
     : mExpatParser(nullptr),
       mInCData(false),
       mInInternalSubset(false),
       mInExternalDTD(false),
       mMadeFinalCallToExpat(false),
       mIsFinalChunk(false),
       mInternalState(NS_OK),
       mExpatBuffered(0),
+      mTagDepth(0),
       mCatalogData(nullptr),
       mInnerWindowID(0) {}
 
 nsExpatDriver::~nsExpatDriver() {
   if (mExpatParser) {
     XML_ParserFree(mExpatParser);
   }
 }
@@ -268,16 +274,26 @@ void nsExpatDriver::HandleStartElement(v
   // ourselves.
   uint32_t attrArrayLength;
   for (attrArrayLength = XML_GetSpecifiedAttributeCount(self->mExpatParser);
        aAtts[attrArrayLength]; attrArrayLength += 2) {
     // Just looping till we find out what the length is
   }
 
   if (self->mSink) {
+    // We store the tagdepth in a PRUint16, so make sure the limit fits in a
+    // PRUint16.
+    static_assert(sMaxXMLTreeDepth <=
+                  mozilla::MaxValue<decltype(nsExpatDriver::mTagDepth)>::value);
+
+    if (++self->mTagDepth > sMaxXMLTreeDepth) {
+      self->MaybeStopParser(NS_ERROR_HTMLPARSER_HIERARCHYTOODEEP);
+      return;
+    }
+
     nsresult rv = self->mSink->HandleStartElement(
         aName, aAtts, attrArrayLength,
         XML_GetCurrentLineNumber(self->mExpatParser),
         XML_GetCurrentColumnNumber(self->mExpatParser));
     self->MaybeStopParser(rv);
   }
 }
 
@@ -321,16 +337,17 @@ void nsExpatDriver::HandleEndElement(voi
   nsExpatDriver* self = static_cast<nsExpatDriver*>(aUserData);
 
   NS_ASSERTION(self->mSink, "content sink not found!");
   NS_ASSERTION(self->mInternalState != NS_ERROR_HTMLPARSER_BLOCK,
                "Shouldn't block from HandleStartElement.");
 
   if (self->mSink && self->mInternalState != NS_ERROR_HTMLPARSER_STOPPARSING) {
     nsresult rv = self->mSink->HandleEndElement(aName);
+    --self->mTagDepth;
     self->MaybeStopParser(rv);
   }
 }
 
 /* static */
 void nsExpatDriver::HandleEndElementForSystemPrincipal(void* aUserData,
                                                        const char16_t* aName) {
   nsExpatDriver* self = static_cast<nsExpatDriver*>(aUserData);
--- a/parser/htmlparser/nsExpatDriver.h
+++ b/parser/htmlparser/nsExpatDriver.h
@@ -108,16 +108,18 @@ class nsExpatDriver : public nsIDTD, pub
   // Necko
   bool mIsFinalChunk;
 
   nsresult mInternalState;
 
   // The length of the data in Expat's buffer (in number of PRUnichars).
   uint32_t mExpatBuffered;
 
+  uint16_t mTagDepth;
+
   // These sinks all refer the same conceptual object. mOriginalSink is
   // identical with the nsIContentSink* passed to WillBuildModel, and exists
   // only to avoid QI-ing back to nsIContentSink*.
   nsCOMPtr<nsIContentSink> mOriginalSink;
   nsCOMPtr<nsIExpatSink> mSink;
 
   const nsCatalogData* mCatalogData;  // weak
   nsString mURISpec;
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/tests/xml/parsedepth.html
@@ -0,0 +1,62 @@
+<!doctype html>
+<meta charset=utf-8>
+<title></title>
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<script>
+
+function parseBlob(blob) {
+  return new Promise(resolve => {
+    let xhr = new XMLHttpRequest();
+    xhr.open("GET", URL.createObjectURL(blob));
+    xhr.onload = () => {
+      resolve(xhr.responseXML);
+    }
+    xhr.send();
+  });
+}
+
+promise_test(async (t) => {
+  // Most browser engines, including Gecko, use 5000 as the limit, so test a
+  // range around that.
+  const cutoff = 5000;
+
+  let minDepth = cutoff - 100;
+  let maxDepth = cutoff + 100;
+
+  // Generate a string with elements nested maxDepth deep.
+  const openTag = "<x>";
+  const closeTag = "</x>";
+  let xml = openTag.repeat(maxDepth) + closeTag.repeat(maxDepth);
+
+  // Compute where we change from opening to closing tags.
+  const middle = maxDepth * openTag.length;
+
+  // Create a blob around the string.
+  let blob = new Blob([xml], { type: "application/xml" });
+
+  while (minDepth < maxDepth) {
+    // Try to parse a number of nested tags between minDepth and maxDepth.
+    let test = Math.ceil((minDepth + maxDepth) / 2);
+
+    // We need the number of opening and closing tags to be equal to the number
+    // that we calculated above.
+    let slice = blob.slice(middle - (test * openTag.length),
+                           middle + (test * closeTag.length));
+
+    let responseXML = await parseBlob(slice);
+
+    // Move either minDepth or maxDepth so that the actual limit is still in the
+    // range of [minDepth-maxDepth].
+    if (responseXML) {
+      // Depth is ok.
+      minDepth = test;
+    } else {
+      maxDepth = test - 1;
+    }
+  }
+  assert_equals(minDepth, maxDepth);
+  assert_equals(minDepth, cutoff);
+},"Parsing XML fails when the nesting depth is 5000");
+
+</script>