Backed out changeset 16c03995ac55 (bug 1539759) for causing Buffer Overflow in nsExpatDriver.cpp
authorMihai Alexandru Michis <malexandru@mozilla.com>
Mon, 13 May 2019 18:16:39 +0300
changeset 532435 ed8137fa8e4ad6b7a9c8c64a51fdcf2b6be3ce2c
parent 532434 4750c8223f658b0abf51974d1a88af0e2070a44b
child 532436 7e851ac3cb03c9c1c1224d5a4da9c7e0f3c5c280
push id11268
push usercsabou@mozilla.com
push dateTue, 14 May 2019 15:24:22 +0000
treeherdermozilla-beta@5fb7fcd568d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1539759
milestone68.0a1
backs out16c03995ac55ce131282881c7c5d31ee6061441e
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
Backed out changeset 16c03995ac55 (bug 1539759) for causing Buffer Overflow in nsExpatDriver.cpp
parser/expat/lib/expat.h
parser/expat/lib/xmlparse.c
parser/htmlparser/nsExpatDriver.cpp
parser/htmlparser/nsExpatDriver.h
--- a/parser/expat/lib/expat.h
+++ b/parser/expat/lib/expat.h
@@ -1051,18 +1051,13 @@ XML_GetFeatureList(void);
 #define XML_MINOR_VERSION 2
 #define XML_MICRO_VERSION 1
 
 /* BEGIN MOZILLA CHANGE (Report opening tag of mismatched closing tag) */
 XMLPARSEAPI(const XML_Char*)
 MOZ_XML_GetMismatchedTag(XML_Parser parser);
 /* END MOZILLA CHANGE */
 
-/* BEGIN MOZILLA CHANGE (Report whether the parser is currently expanding an entity) */
-XMLPARSEAPI(XML_Bool)
-MOZ_XML_ProcessingEntityValue(XML_Parser parser);
-/* END MOZILLA CHANGE */
-
 #ifdef __cplusplus
 }
 #endif
 
 #endif /* not Expat_INCLUDED */
--- a/parser/expat/lib/xmlparse.c
+++ b/parser/expat/lib/xmlparse.c
@@ -2457,23 +2457,16 @@ XML_GetFeatureList(void)
 /* BEGIN MOZILLA CHANGE (Report opening tag of mismatched closing tag) */
 const XML_Char * XMLCALL
 MOZ_XML_GetMismatchedTag(XML_Parser parser)
 {
   return mismatch;
 }
 /* END MOZILLA CHANGE */
 
-/* BEGIN MOZILLA CHANGE (Report whether the parser is currently expanding an entity) */
-XML_Bool XMLCALL
-MOZ_XML_ProcessingEntityValue(XML_Parser parser) {
-  return openInternalEntities != NULL;
-}
-/* END MOZILLA CHANGE */
-
 /* Initially tag->rawName always points into the parse buffer;
    for those TAG instances opened while the current parse buffer was
    processed, and not yet closed, we need to store tag->rawName in a more
    permanent location, since the parse buffer is about to be discarded.
 */
 static XML_Bool
 storeRawNames(XML_Parser parser)
 {
--- a/parser/htmlparser/nsExpatDriver.cpp
+++ b/parser/htmlparser/nsExpatDriver.cpp
@@ -49,16 +49,31 @@ static void Driver_HandleXMLDeclaration(
                                         int aStandalone) {
   NS_ASSERTION(aUserData, "expat driver should exist");
   if (aUserData) {
     nsExpatDriver* driver = static_cast<nsExpatDriver*>(aUserData);
     driver->HandleXMLDeclaration(aVersion, aEncoding, aStandalone);
   }
 }
 
+static void Driver_HandleStartElement(void* aUserData, const XML_Char* aName,
+                                      const XML_Char** aAtts) {
+  NS_ASSERTION(aUserData, "expat driver should exist");
+  if (aUserData) {
+    static_cast<nsExpatDriver*>(aUserData)->HandleStartElement(aName, aAtts);
+  }
+}
+
+static void Driver_HandleEndElement(void* aUserData, const XML_Char* aName) {
+  NS_ASSERTION(aUserData, "expat driver should exist");
+  if (aUserData) {
+    static_cast<nsExpatDriver*>(aUserData)->HandleEndElement(aName);
+  }
+}
+
 static void Driver_HandleCharacterData(void* aUserData, const XML_Char* aData,
                                        int aLength) {
   NS_ASSERTION(aUserData, "expat driver should exist");
   if (aUserData) {
     nsExpatDriver* driver = static_cast<nsExpatDriver*>(aUserData);
     driver->HandleCharacterData(aData, uint32_t(aLength));
   }
 }
@@ -247,114 +262,51 @@ nsExpatDriver::nsExpatDriver()
       mInnerWindowID(0) {}
 
 nsExpatDriver::~nsExpatDriver() {
   if (mExpatParser) {
     XML_ParserFree(mExpatParser);
   }
 }
 
-/* static */
-void nsExpatDriver::HandleStartElement(void* aUserData, const char16_t* aName,
-                                       const char16_t** aAtts) {
-  nsExpatDriver* self = static_cast<nsExpatDriver*>(aUserData);
-
-  NS_ASSERTION(self->mSink, "content sink not found!");
+nsresult nsExpatDriver::HandleStartElement(const char16_t* aValue,
+                                           const char16_t** aAtts) {
+  NS_ASSERTION(mSink, "content sink not found!");
 
   // Calculate the total number of elements in aAtts.
   // XML_GetSpecifiedAttributeCount will only give us the number of specified
   // attrs (twice that number, actually), so we have to check for default attrs
   // ourselves.
   uint32_t attrArrayLength;
-  for (attrArrayLength = XML_GetSpecifiedAttributeCount(self->mExpatParser);
+  for (attrArrayLength = XML_GetSpecifiedAttributeCount(mExpatParser);
        aAtts[attrArrayLength]; attrArrayLength += 2) {
     // Just looping till we find out what the length is
   }
 
-  if (self->mSink) {
-    nsresult rv = self->mSink->HandleStartElement(
-        aName, aAtts, attrArrayLength,
-        XML_GetCurrentLineNumber(self->mExpatParser),
-        XML_GetCurrentColumnNumber(self->mExpatParser));
-    self->MaybeStopParser(rv);
+  if (mSink) {
+    nsresult rv = mSink->HandleStartElement(
+        aValue, aAtts, attrArrayLength, XML_GetCurrentLineNumber(mExpatParser),
+        XML_GetCurrentColumnNumber(mExpatParser));
+    MaybeStopParser(rv);
   }
-}
-
-static nsresult AppendErrorPointer(const int32_t aColNumber,
-                                   const char16_t* aSourceLine,
-                                   nsString& aSourceString) {
-  aSourceString.Append(char16_t('\n'));
-
-  // Last character will be '^'.
-  int32_t last = aColNumber - 1;
-  int32_t i;
-  uint32_t minuses = 0;
-  for (i = 0; i < last; ++i) {
-    if (aSourceLine[i] == '\t') {
-      // Since this uses |white-space: pre;| a tab stop equals 8 spaces.
-      uint32_t add = 8 - (minuses % 8);
-      aSourceString.AppendASCII("--------", add);
-      minuses += add;
-    } else {
-      aSourceString.Append(char16_t('-'));
-      ++minuses;
-    }
-  }
-  aSourceString.Append(char16_t('^'));
 
   return NS_OK;
 }
 
-/* static */
-void nsExpatDriver::HandleStartElementForSystemPrincipal(
-    void* aUserData, const char16_t* aName, const char16_t** aAtts) {
-  nsExpatDriver* self = static_cast<nsExpatDriver*>(aUserData);
-
-  if (!MOZ_XML_ProcessingEntityValue(self->mExpatParser)) {
-    HandleStartElement(aUserData, aName, aAtts);
-  } else {
-    nsCOMPtr<Document> doc =
-        do_QueryInterface(self->mOriginalSink->GetTarget());
-
-    // Adjust the column number so that it is one based rather than zero based.
-    uint32_t colNumber = XML_GetCurrentColumnNumber(self->mExpatParser) + 1;
-    uint32_t lineNumber = XML_GetCurrentLineNumber(self->mExpatParser);
-
-    nsAutoString sourceText(self->mLastLine);
-    AppendErrorPointer(colNumber, self->mLastLine.get(), sourceText);
-
-    nsContentUtils::ReportToConsoleNonLocalized(
-        NS_LITERAL_STRING("Ignored element created from entity value."),
-        nsIScriptError::warningFlag, NS_LITERAL_CSTRING("XML Document"), doc,
-        nullptr, sourceText, lineNumber, colNumber);
-  }
-}
-
-/* static */
-void nsExpatDriver::HandleEndElement(void* aUserData, const char16_t* aName) {
-  nsExpatDriver* self = static_cast<nsExpatDriver*>(aUserData);
-
-  NS_ASSERTION(self->mSink, "content sink not found!");
-  NS_ASSERTION(self->mInternalState != NS_ERROR_HTMLPARSER_BLOCK,
+nsresult nsExpatDriver::HandleEndElement(const char16_t* aValue) {
+  NS_ASSERTION(mSink, "content sink not found!");
+  NS_ASSERTION(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->MaybeStopParser(rv);
+  if (mSink && mInternalState != NS_ERROR_HTMLPARSER_STOPPARSING) {
+    nsresult rv = mSink->HandleEndElement(aValue);
+    MaybeStopParser(rv);
   }
-}
 
-/* static */
-void nsExpatDriver::HandleEndElementForSystemPrincipal(void* aUserData,
-                                                       const char16_t* aName) {
-  nsExpatDriver* self = static_cast<nsExpatDriver*>(aUserData);
-
-  if (!MOZ_XML_ProcessingEntityValue(self->mExpatParser)) {
-    HandleEndElement(aUserData, aName);
-  }
+  return NS_OK;
 }
 
 nsresult nsExpatDriver::HandleCharacterData(const char16_t* aValue,
                                             const uint32_t aLength) {
   NS_ASSERTION(mSink, "content sink not found!");
 
   if (mInCData) {
     if (!mCDataText.Append(aValue, aLength, fallible)) {
@@ -689,16 +641,41 @@ static nsresult CreateErrorText(const ch
   NS_ENSURE_SUCCESS(rv, rv);
 
   // XML Parsing Error: %1$S\nLocation: %2$S\nLine Number %3$u, Column %4$u:
   nsTextFormatter::ssprintf(aErrorString, msg.get(), aDescription, aSourceURL,
                             aLineNumber, aColNumber);
   return NS_OK;
 }
 
+static nsresult AppendErrorPointer(const int32_t aColNumber,
+                                   const char16_t* aSourceLine,
+                                   nsString& aSourceString) {
+  aSourceString.Append(char16_t('\n'));
+
+  // Last character will be '^'.
+  int32_t last = aColNumber - 1;
+  int32_t i;
+  uint32_t minuses = 0;
+  for (i = 0; i < last; ++i) {
+    if (aSourceLine[i] == '\t') {
+      // Since this uses |white-space: pre;| a tab stop equals 8 spaces.
+      uint32_t add = 8 - (minuses % 8);
+      aSourceString.AppendASCII("--------", add);
+      minuses += add;
+    } else {
+      aSourceString.Append(char16_t('-'));
+      ++minuses;
+    }
+  }
+  aSourceString.Append(char16_t('^'));
+
+  return NS_OK;
+}
+
 nsresult nsExpatDriver::HandleError() {
   int32_t code = XML_GetErrorCode(mExpatParser);
   NS_ASSERTION(code > XML_ERROR_NONE, "unexpected XML error code");
 
   // Map Expat error code to an error string
   // XXX Deal with error returns.
   nsAutoString description;
   nsParserMsgUtils::GetLocalizedStringByID(XMLPARSER_PROPERTIES, code,
@@ -1059,22 +1036,18 @@ nsExpatDriver::WillBuildModel(const CPar
     }
     if (inner) {
       mInnerWindowID = inner->WindowID();
     }
   }
 
   // Set up the callbacks
   XML_SetXmlDeclHandler(mExpatParser, Driver_HandleXMLDeclaration);
-  if (doc && doc->NodePrincipal()->IsSystemPrincipal()) {
-    XML_SetElementHandler(mExpatParser, HandleStartElementForSystemPrincipal,
-                          HandleEndElementForSystemPrincipal);
-  } else {
-    XML_SetElementHandler(mExpatParser, HandleStartElement, HandleEndElement);
-  }
+  XML_SetElementHandler(mExpatParser, Driver_HandleStartElement,
+                        Driver_HandleEndElement);
   XML_SetCharacterDataHandler(mExpatParser, Driver_HandleCharacterData);
   XML_SetProcessingInstructionHandler(mExpatParser,
                                       Driver_HandleProcessingInstruction);
   XML_SetDefaultHandlerExpand(mExpatParser, Driver_HandleDefault);
   XML_SetExternalEntityRefHandler(
       mExpatParser,
       (XML_ExternalEntityRefHandler)Driver_HandleExternalEntityRef);
   XML_SetExternalEntityRefHandlerArg(mExpatParser, this);
--- a/parser/htmlparser/nsExpatDriver.h
+++ b/parser/htmlparser/nsExpatDriver.h
@@ -28,24 +28,18 @@ class nsExpatDriver : public nsIDTD, pub
   NS_DECL_NSITOKENIZER
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsExpatDriver, nsIDTD)
 
   nsExpatDriver();
 
   int HandleExternalEntityRef(const char16_t* aOpenEntityNames,
                               const char16_t* aBase, const char16_t* aSystemId,
                               const char16_t* aPublicId);
-  static void HandleStartElement(void* aUserData, const char16_t* aName,
-                                 const char16_t** aAtts);
-  static void HandleStartElementForSystemPrincipal(void* aUserData,
-                                                   const char16_t* aName,
-                                                   const char16_t** aAtts);
-  static void HandleEndElement(void* aUserData, const char16_t* aName);
-  static void HandleEndElementForSystemPrincipal(void* aUserData,
-                                                 const char16_t* aName);
+  nsresult HandleStartElement(const char16_t* aName, const char16_t** aAtts);
+  nsresult HandleEndElement(const char16_t* aName);
   nsresult HandleCharacterData(const char16_t* aCData, const uint32_t aLength);
   nsresult HandleComment(const char16_t* aName);
   nsresult HandleProcessingInstruction(const char16_t* aTarget,
                                        const char16_t* aData);
   nsresult HandleXMLDeclaration(const char16_t* aVersion,
                                 const char16_t* aEncoding, int32_t aStandalone);
   nsresult HandleDefault(const char16_t* aData, const uint32_t aLength);
   nsresult HandleStartCdataSection();