Bug 1824892: Fix tainting use in the nsExpatDriver r=glandium
Differential Revision: https://phabricator.services.mozilla.com/D175433
This commit is contained in:
@@ -85,9 +85,17 @@ static const uint16_t sMaxXMLTreeDepth = 5000;
|
|||||||
->invoke_sandbox_function(foo, mExpatParser, ##__VA_ARGS__) \
|
->invoke_sandbox_function(foo, mExpatParser, ##__VA_ARGS__) \
|
||||||
.copy_and_verify(verifier)
|
.copy_and_verify(verifier)
|
||||||
|
|
||||||
|
#define RLBOX_EXPAT_CALL(foo, ...) \
|
||||||
|
aSandbox.invoke_sandbox_function(foo, self->mExpatParser, ##__VA_ARGS__)
|
||||||
|
|
||||||
#define RLBOX_EXPAT_MCALL(foo, ...) \
|
#define RLBOX_EXPAT_MCALL(foo, ...) \
|
||||||
Sandbox()->invoke_sandbox_function(foo, mExpatParser, ##__VA_ARGS__)
|
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 */
|
/* safe_unverified is used whenever it's safe to not use a validator */
|
||||||
template <typename T>
|
template <typename T>
|
||||||
static T safe_unverified(T val) {
|
static T safe_unverified(T val) {
|
||||||
@@ -421,19 +429,25 @@ void nsExpatDriver::HandleStartElement(rlbox_sandbox_expat& aSandbox,
|
|||||||
// XML_GetSpecifiedAttributeCount will only give us the number of specified
|
// XML_GetSpecifiedAttributeCount will only give us the number of specified
|
||||||
// attrs (twice that number, actually), so we have to check for default
|
// attrs (twice that number, actually), so we have to check for default
|
||||||
// attrs ourselves.
|
// attrs ourselves.
|
||||||
int count = RLBOX_EXPAT_SAFE_CALL(MOZ_XML_GetSpecifiedAttributeCount,
|
tainted_expat<int> count =
|
||||||
safe_unverified<int>);
|
RLBOX_EXPAT_CALL(MOZ_XML_GetSpecifiedAttributeCount);
|
||||||
MOZ_RELEASE_ASSERT(count >= 0, "Unexpected attribute count");
|
MOZ_RELEASE_ASSERT_TAINTED(count >= 0, "Unexpected attribute count");
|
||||||
uint32_t attrArrayLength;
|
|
||||||
for (attrArrayLength = count;
|
tainted_expat<uint64_t> attrArrayLengthTainted;
|
||||||
(aAttrs[attrArrayLength] != nullptr)
|
for (attrArrayLengthTainted = rlbox::sandbox_static_cast<uint64_t>(count);
|
||||||
|
(aAttrs[attrArrayLengthTainted] != nullptr)
|
||||||
.unverified_safe_because("Bad length is checked later");
|
.unverified_safe_because("Bad length is checked later");
|
||||||
attrArrayLength += 2) {
|
attrArrayLengthTainted += 2) {
|
||||||
// Just looping till we find out what the length is
|
// Just looping till we find out what the length is
|
||||||
}
|
}
|
||||||
// A malicious length could result in an overflow when we allocate aAttrs
|
|
||||||
// and then access elements of the array.
|
uint32_t attrArrayLength =
|
||||||
MOZ_RELEASE_ASSERT(attrArrayLength < UINT32_MAX, "Overflow attempt");
|
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(value < UINT32_MAX, "Overflow attempt");
|
||||||
|
return value;
|
||||||
|
});
|
||||||
|
|
||||||
// Copy tainted aAttrs from sandbox
|
// Copy tainted aAttrs from sandbox
|
||||||
AllocAttrs allocAttrs;
|
AllocAttrs allocAttrs;
|
||||||
@@ -486,11 +500,10 @@ void nsExpatDriver::HandleStartElementForSystemPrincipal(
|
|||||||
|
|
||||||
// Adjust the column number so that it is one based rather than zero
|
// Adjust the column number so that it is one based rather than zero
|
||||||
// based.
|
// based.
|
||||||
uint32_t colNumber = RLBOX_EXPAT_SAFE_CALL(MOZ_XML_GetCurrentColumnNumber,
|
tainted_expat<XML_Size> colNumber =
|
||||||
safe_unverified<XML_Size>) +
|
RLBOX_EXPAT_CALL(MOZ_XML_GetCurrentColumnNumber) + 1;
|
||||||
1;
|
tainted_expat<XML_Size> lineNumber =
|
||||||
uint32_t lineNumber = RLBOX_EXPAT_SAFE_CALL(MOZ_XML_GetCurrentLineNumber,
|
RLBOX_EXPAT_CALL(MOZ_XML_GetCurrentLineNumber);
|
||||||
safe_unverified<XML_Size>);
|
|
||||||
|
|
||||||
int32_t nameSpaceID;
|
int32_t nameSpaceID;
|
||||||
RefPtr<nsAtom> prefix, localName;
|
RefPtr<nsAtom> prefix, localName;
|
||||||
@@ -509,7 +522,8 @@ void nsExpatDriver::HandleStartElementForSystemPrincipal(
|
|||||||
|
|
||||||
nsContentUtils::ReportToConsoleNonLocalized(
|
nsContentUtils::ReportToConsoleNonLocalized(
|
||||||
error, nsIScriptError::warningFlag, "XML Document"_ns, doc, nullptr,
|
error, nsIScriptError::warningFlag, "XML Document"_ns, doc, nullptr,
|
||||||
u""_ns, lineNumber, colNumber);
|
u""_ns, lineNumber.unverified_safe_because(RLBOX_SAFE_PRINT),
|
||||||
|
colNumber.unverified_safe_because(RLBOX_SAFE_PRINT));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -930,8 +944,8 @@ nsresult nsExpatDriver::OpenInputStreamFromExternalDTD(const char16_t* aFPIStr,
|
|||||||
|
|
||||||
static nsresult CreateErrorText(const char16_t* aDescription,
|
static nsresult CreateErrorText(const char16_t* aDescription,
|
||||||
const char16_t* aSourceURL,
|
const char16_t* aSourceURL,
|
||||||
const uint32_t aLineNumber,
|
tainted_expat<XML_Size> aLineNumber,
|
||||||
const uint32_t aColNumber,
|
tainted_expat<XML_Size> aColNumber,
|
||||||
nsString& aErrorString, bool spoofEnglish) {
|
nsString& aErrorString, bool spoofEnglish) {
|
||||||
aErrorString.Truncate();
|
aErrorString.Truncate();
|
||||||
|
|
||||||
@@ -942,19 +956,30 @@ static nsresult CreateErrorText(const char16_t* aDescription,
|
|||||||
NS_ENSURE_SUCCESS(rv, rv);
|
NS_ENSURE_SUCCESS(rv, rv);
|
||||||
|
|
||||||
// XML Parsing Error: %1$S\nLocation: %2$S\nLine Number %3$u, Column %4$u:
|
// XML Parsing Error: %1$S\nLocation: %2$S\nLine Number %3$u, Column %4$u:
|
||||||
nsTextFormatter::ssprintf(aErrorString, msg.get(), aDescription, aSourceURL,
|
nsTextFormatter::ssprintf(
|
||||||
aLineNumber, aColNumber);
|
aErrorString, msg.get(), aDescription, aSourceURL,
|
||||||
|
aLineNumber.unverified_safe_because(RLBOX_SAFE_PRINT),
|
||||||
|
aColNumber.unverified_safe_because(RLBOX_SAFE_PRINT));
|
||||||
return NS_OK;
|
return NS_OK;
|
||||||
}
|
}
|
||||||
|
|
||||||
static nsresult AppendErrorPointer(const int32_t aColNumber,
|
static nsresult AppendErrorPointer(tainted_expat<XML_Size> aColNumber,
|
||||||
const char16_t* aSourceLine,
|
const char16_t* aSourceLine,
|
||||||
|
size_t aSourceLineLength,
|
||||||
nsString& aSourceString) {
|
nsString& aSourceString) {
|
||||||
aSourceString.Append(char16_t('\n'));
|
aSourceString.Append(char16_t('\n'));
|
||||||
|
|
||||||
|
MOZ_RELEASE_ASSERT_TAINTED(aColNumber != static_cast<XML_Size>(0),
|
||||||
|
"Unexpected value of column");
|
||||||
|
|
||||||
// Last character will be '^'.
|
// Last character will be '^'.
|
||||||
int32_t last = aColNumber - 1;
|
XML_Size last = (aColNumber - 1).copy_and_verify([&](XML_Size val) {
|
||||||
int32_t i;
|
MOZ_RELEASE_ASSERT(val <= aSourceLineLength,
|
||||||
|
"Unexpected value of last column");
|
||||||
|
return val;
|
||||||
|
});
|
||||||
|
|
||||||
|
XML_Size i;
|
||||||
uint32_t minuses = 0;
|
uint32_t minuses = 0;
|
||||||
for (i = 0; i < last; ++i) {
|
for (i = 0; i < last; ++i) {
|
||||||
if (aSourceLine[i] == '\t') {
|
if (aSourceLine[i] == '\t') {
|
||||||
@@ -973,7 +998,8 @@ static nsresult AppendErrorPointer(const int32_t aColNumber,
|
|||||||
}
|
}
|
||||||
|
|
||||||
nsresult nsExpatDriver::HandleError() {
|
nsresult nsExpatDriver::HandleError() {
|
||||||
int32_t code = RLBOX_EXPAT_SAFE_MCALL(MOZ_XML_GetErrorCode, error_verifier);
|
int32_t code =
|
||||||
|
RLBOX_EXPAT_MCALL(MOZ_XML_GetErrorCode).copy_and_verify(error_verifier);
|
||||||
|
|
||||||
// Map Expat error code to an error string
|
// Map Expat error code to an error string
|
||||||
// XXX Deal with error returns.
|
// XXX Deal with error returns.
|
||||||
@@ -1037,17 +1063,24 @@ nsresult nsExpatDriver::HandleError() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Adjust the column number so that it is one based rather than zero based.
|
// Adjust the column number so that it is one based rather than zero based.
|
||||||
uint32_t colNumber = RLBOX_EXPAT_SAFE_MCALL(MOZ_XML_GetCurrentColumnNumber,
|
tainted_expat<XML_Size> colNumber =
|
||||||
safe_unverified<XML_Size>) +
|
RLBOX_EXPAT_MCALL(MOZ_XML_GetCurrentColumnNumber) + 1;
|
||||||
1;
|
tainted_expat<XML_Size> lineNumber =
|
||||||
uint32_t lineNumber = RLBOX_EXPAT_SAFE_MCALL(MOZ_XML_GetCurrentLineNumber,
|
RLBOX_EXPAT_MCALL(MOZ_XML_GetCurrentLineNumber);
|
||||||
safe_unverified<XML_Size>);
|
|
||||||
const XML_Char* expatBase =
|
// Copy out the two character bufer that holds the expatBase
|
||||||
|
const std::unique_ptr<XML_Char[]> expatBase =
|
||||||
RLBOX_EXPAT_MCALL(MOZ_XML_GetBase)
|
RLBOX_EXPAT_MCALL(MOZ_XML_GetBase)
|
||||||
.copy_and_verify_address(unverified_xml_string);
|
.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);
|
||||||
nsAutoString uri;
|
nsAutoString uri;
|
||||||
nsCOMPtr<nsIURI> baseURI;
|
nsCOMPtr<nsIURI> baseURI;
|
||||||
if (expatBase && (baseURI = GetBaseURI(expatBase))) {
|
if (expatBase && (baseURI = GetBaseURI(expatBase.get()))) {
|
||||||
// Let's ignore if this fails, we're already reporting a parse error.
|
// Let's ignore if this fails, we're already reporting a parse error.
|
||||||
Unused << CopyUTF8toUTF16(baseURI->GetSpecOrDefault(), uri, fallible);
|
Unused << CopyUTF8toUTF16(baseURI->GetSpecOrDefault(), uri, fallible);
|
||||||
}
|
}
|
||||||
@@ -1056,7 +1089,8 @@ nsresult nsExpatDriver::HandleError() {
|
|||||||
errorText, spoofEnglish);
|
errorText, spoofEnglish);
|
||||||
|
|
||||||
nsAutoString sourceText(mLastLine);
|
nsAutoString sourceText(mLastLine);
|
||||||
AppendErrorPointer(colNumber, mLastLine.get(), sourceText);
|
AppendErrorPointer(colNumber, mLastLine.get(), mLastLine.Length(),
|
||||||
|
sourceText);
|
||||||
|
|
||||||
if (doc && nsContentUtils::IsChromeDoc(doc)) {
|
if (doc && nsContentUtils::IsChromeDoc(doc)) {
|
||||||
nsCString path = doc->GetDocumentURI()->GetSpecOrDefault();
|
nsCString path = doc->GetDocumentURI()->GetSpecOrDefault();
|
||||||
@@ -1074,7 +1108,11 @@ nsresult nsExpatDriver::HandleError() {
|
|||||||
mozilla::Telemetry::EventExtraEntry{"error_code"_ns,
|
mozilla::Telemetry::EventExtraEntry{"error_code"_ns,
|
||||||
nsPrintfCString("%u", code)},
|
nsPrintfCString("%u", code)},
|
||||||
mozilla::Telemetry::EventExtraEntry{
|
mozilla::Telemetry::EventExtraEntry{
|
||||||
"location"_ns, nsPrintfCString("%u:%u", lineNumber, colNumber)},
|
"location"_ns,
|
||||||
|
nsPrintfCString(
|
||||||
|
"%lu:%lu",
|
||||||
|
lineNumber.unverified_safe_because(RLBOX_SAFE_PRINT),
|
||||||
|
colNumber.unverified_safe_because(RLBOX_SAFE_PRINT))},
|
||||||
mozilla::Telemetry::EventExtraEntry{
|
mozilla::Telemetry::EventExtraEntry{
|
||||||
"last_line"_ns, NS_ConvertUTF16toUTF8(mLastLine)},
|
"last_line"_ns, NS_ConvertUTF16toUTF8(mLastLine)},
|
||||||
mozilla::Telemetry::EventExtraEntry{
|
mozilla::Telemetry::EventExtraEntry{
|
||||||
@@ -1096,7 +1134,9 @@ nsresult nsExpatDriver::HandleError() {
|
|||||||
nsresult rv = NS_ERROR_FAILURE;
|
nsresult rv = NS_ERROR_FAILURE;
|
||||||
if (serr) {
|
if (serr) {
|
||||||
rv = serr->InitWithSourceURI(
|
rv = serr->InitWithSourceURI(
|
||||||
errorText, mURIs.SafeElementAt(0), mLastLine, lineNumber, colNumber,
|
errorText, mURIs.SafeElementAt(0), mLastLine,
|
||||||
|
lineNumber.unverified_safe_because(RLBOX_SAFE_PRINT),
|
||||||
|
colNumber.unverified_safe_because(RLBOX_SAFE_PRINT),
|
||||||
nsIScriptError::errorFlag, "malformed-xml", mInnerWindowID);
|
nsIScriptError::errorFlag, "malformed-xml", mInnerWindowID);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user