Skip to content
Snippets Groups Projects
Commit 00a30ad5 authored by Narcis Beleuzu's avatar Narcis Beleuzu
Browse files

Backed out changeset 4b412f721080 (bug 1824892) for crashes on nsExpatDriver.cpp . CLOSED TREE

parent d39c9ba2
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
parser/htmlparser/nsExpatDriver.cpp
+ 35
− 75
View file @ 00a30ad5
......@@ -85,17 +85,9 @@ static const uint16_t sMaxXMLTreeDepth = 5000;
->invoke_sandbox_function(foo, mExpatParser, ##__VA_ARGS__) \
.copy_and_verify(verifier)
#define RLBOX_EXPAT_CALL(foo, ...) \
aSandbox.invoke_sandbox_function(foo, self->mExpatParser, ##__VA_ARGS__)
#define RLBOX_EXPAT_MCALL(foo, ...) \
Sandbox()->invoke_sandbox_function(foo, mExpatParser, ##__VA_ARGS__)
#define RLBOX_SAFE_PRINT "Value used only for printing"
#define MOZ_RELEASE_ASSERT_TAINTED(cond, ...) \
MOZ_RELEASE_ASSERT((cond).unverified_safe_because("Sanity check"), \
##__VA_ARGS__)
/* safe_unverified is used whenever it's safe to not use a validator */
template <typename T>
static T safe_unverified(T val) {
......@@ -429,25 +421,19 @@ void nsExpatDriver::HandleStartElement(rlbox_sandbox_expat& aSandbox,
// XML_GetSpecifiedAttributeCount will only give us the number of specified
// attrs (twice that number, actually), so we have to check for default
// attrs ourselves.
tainted_expat<int> count =
RLBOX_EXPAT_CALL(MOZ_XML_GetSpecifiedAttributeCount);
MOZ_RELEASE_ASSERT_TAINTED(count >= 0, "Unexpected attribute count");
tainted_expat<uint64_t> attrArrayLengthTainted;
for (attrArrayLengthTainted = rlbox::sandbox_static_cast<uint64_t>(count);
(aAttrs[attrArrayLengthTainted] != nullptr)
int count = RLBOX_EXPAT_SAFE_CALL(MOZ_XML_GetSpecifiedAttributeCount,
safe_unverified<int>);
MOZ_RELEASE_ASSERT(count >= 0, "Unexpected attribute count");
uint32_t attrArrayLength;
for (attrArrayLength = count;
(aAttrs[attrArrayLength] != nullptr)
.unverified_safe_because("Bad length is checked later");
attrArrayLengthTainted += 2) {
attrArrayLength += 2) {
// Just looping till we find out what the length is
}
uint32_t attrArrayLength =
attrArrayLengthTainted.copy_and_verify([&](uint64_t value) {
// A malicious length could result in an overflow when we allocate
// aAttrs and then access elements of the array.
MOZ_RELEASE_ASSERT(attrArrayLength < UINT32_MAX, "Overflow attempt");
return value;
});
// A malicious length could result in an overflow when we allocate aAttrs
// and then access elements of the array.
MOZ_RELEASE_ASSERT(attrArrayLength < UINT32_MAX, "Overflow attempt");
// Copy tainted aAttrs from sandbox
AllocAttrs allocAttrs;
......@@ -500,10 +486,11 @@ void nsExpatDriver::HandleStartElementForSystemPrincipal(
// Adjust the column number so that it is one based rather than zero
// based.
tainted_expat<XML_Size> colNumber =
RLBOX_EXPAT_CALL(MOZ_XML_GetCurrentColumnNumber) + 1;
tainted_expat<XML_Size> lineNumber =
RLBOX_EXPAT_CALL(MOZ_XML_GetCurrentLineNumber);
uint32_t colNumber = RLBOX_EXPAT_SAFE_CALL(MOZ_XML_GetCurrentColumnNumber,
safe_unverified<XML_Size>) +
1;
uint32_t lineNumber = RLBOX_EXPAT_SAFE_CALL(MOZ_XML_GetCurrentLineNumber,
safe_unverified<XML_Size>);
int32_t nameSpaceID;
RefPtr<nsAtom> prefix, localName;
......@@ -522,8 +509,7 @@ void nsExpatDriver::HandleStartElementForSystemPrincipal(
nsContentUtils::ReportToConsoleNonLocalized(
error, nsIScriptError::warningFlag, "XML Document"_ns, doc, nullptr,
u""_ns, lineNumber.unverified_safe_because(RLBOX_SAFE_PRINT),
colNumber.unverified_safe_because(RLBOX_SAFE_PRINT));
u""_ns, lineNumber, colNumber);
}
}
......@@ -944,8 +930,8 @@ nsresult nsExpatDriver::OpenInputStreamFromExternalDTD(const char16_t* aFPIStr,
static nsresult CreateErrorText(const char16_t* aDescription,
const char16_t* aSourceURL,
tainted_expat<XML_Size> aLineNumber,
tainted_expat<XML_Size> aColNumber,
const uint32_t aLineNumber,
const uint32_t aColNumber,
nsString& aErrorString, bool spoofEnglish) {
aErrorString.Truncate();
......@@ -956,30 +942,19 @@ static nsresult CreateErrorText(const char16_t* aDescription,
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.unverified_safe_because(RLBOX_SAFE_PRINT),
aColNumber.unverified_safe_because(RLBOX_SAFE_PRINT));
nsTextFormatter::ssprintf(aErrorString, msg.get(), aDescription, aSourceURL,
aLineNumber, aColNumber);
return NS_OK;
}
static nsresult AppendErrorPointer(tainted_expat<XML_Size> aColNumber,
static nsresult AppendErrorPointer(const int32_t aColNumber,
const char16_t* aSourceLine,
size_t aSourceLineLength,
nsString& aSourceString) {
aSourceString.Append(char16_t('\n'));
MOZ_RELEASE_ASSERT_TAINTED(aColNumber != static_cast<XML_Size>(0),
"Unexpected value of column");
// Last character will be '^'.
XML_Size last = (aColNumber - 1).copy_and_verify([&](XML_Size val) {
MOZ_RELEASE_ASSERT(val <= aSourceLineLength,
"Unexpected value of last column");
return val;
});
XML_Size i;
int32_t last = aColNumber - 1;
int32_t i;
uint32_t minuses = 0;
for (i = 0; i < last; ++i) {
if (aSourceLine[i] == '\t') {
......@@ -998,8 +973,7 @@ static nsresult AppendErrorPointer(tainted_expat<XML_Size> aColNumber,
}
nsresult nsExpatDriver::HandleError() {
int32_t code =
RLBOX_EXPAT_MCALL(MOZ_XML_GetErrorCode).copy_and_verify(error_verifier);
int32_t code = RLBOX_EXPAT_SAFE_MCALL(MOZ_XML_GetErrorCode, error_verifier);
// Map Expat error code to an error string
// XXX Deal with error returns.
......@@ -1063,24 +1037,17 @@ nsresult nsExpatDriver::HandleError() {
}
// Adjust the column number so that it is one based rather than zero based.
tainted_expat<XML_Size> colNumber =
RLBOX_EXPAT_MCALL(MOZ_XML_GetCurrentColumnNumber) + 1;
tainted_expat<XML_Size> lineNumber =
RLBOX_EXPAT_MCALL(MOZ_XML_GetCurrentLineNumber);
// Copy out the two character bufer that holds the expatBase
const std::unique_ptr<XML_Char[]> expatBase =
uint32_t colNumber = RLBOX_EXPAT_SAFE_MCALL(MOZ_XML_GetCurrentColumnNumber,
safe_unverified<XML_Size>) +
1;
uint32_t lineNumber = RLBOX_EXPAT_SAFE_MCALL(MOZ_XML_GetCurrentLineNumber,
safe_unverified<XML_Size>);
const XML_Char* expatBase =
RLBOX_EXPAT_MCALL(MOZ_XML_GetBase)
.copy_and_verify_range(
[](std::unique_ptr<XML_Char[]> val) {
// No additional checks needed as this is sent to GetBaseURI
// which checks its inputs
return val;
},
ExpatBaseURI::Length);
.copy_and_verify_address(unverified_xml_string);
nsAutoString uri;
nsCOMPtr<nsIURI> baseURI;
if (expatBase && (baseURI = GetBaseURI(expatBase.get()))) {
if (expatBase && (baseURI = GetBaseURI(expatBase))) {
// Let's ignore if this fails, we're already reporting a parse error.
Unused << CopyUTF8toUTF16(baseURI->GetSpecOrDefault(), uri, fallible);
}
......@@ -1089,8 +1056,7 @@ nsresult nsExpatDriver::HandleError() {
errorText, spoofEnglish);
nsAutoString sourceText(mLastLine);
AppendErrorPointer(colNumber, mLastLine.get(), mLastLine.Length(),
sourceText);
AppendErrorPointer(colNumber, mLastLine.get(), sourceText);
if (doc && nsContentUtils::IsChromeDoc(doc)) {
nsCString path = doc->GetDocumentURI()->GetSpecOrDefault();
......@@ -1108,11 +1074,7 @@ nsresult nsExpatDriver::HandleError() {
mozilla::Telemetry::EventExtraEntry{"error_code"_ns,
nsPrintfCString("%u", code)},
mozilla::Telemetry::EventExtraEntry{
"location"_ns,
nsPrintfCString(
"%lu:%lu",
lineNumber.unverified_safe_because(RLBOX_SAFE_PRINT),
colNumber.unverified_safe_because(RLBOX_SAFE_PRINT))},
"location"_ns, nsPrintfCString("%u:%u", lineNumber, colNumber)},
mozilla::Telemetry::EventExtraEntry{
"last_line"_ns, NS_ConvertUTF16toUTF8(mLastLine)},
mozilla::Telemetry::EventExtraEntry{
......@@ -1134,9 +1096,7 @@ nsresult nsExpatDriver::HandleError() {
nsresult rv = NS_ERROR_FAILURE;
if (serr) {
rv = serr->InitWithSourceURI(
errorText, mURIs.SafeElementAt(0), mLastLine,
lineNumber.unverified_safe_because(RLBOX_SAFE_PRINT),
colNumber.unverified_safe_because(RLBOX_SAFE_PRINT),
errorText, mURIs.SafeElementAt(0), mLastLine, lineNumber, colNumber,
nsIScriptError::errorFlag, "malformed-xml", mInnerWindowID);
}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment