tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 57c3d08ee2555fa6737aff9c7a21309ad057cd16
parent 8b42ef8a83b07cc4a2492dc0cb24df0bbc3f8ecc
Author: Malte Jürgens <maltejur@mozilla.com>
Date:   Wed, 17 Dec 2025 15:36:30 +0000

Bug 2004710 - Check that SRI hash is in base64 format r=tschuster

Differential Revision: https://phabricator.services.mozilla.com/D276183

Diffstat:
Mdom/security/SRIMetadata.cpp | 22+++++++++++++++++-----
Mdom/security/nsCSPParser.cpp | 11++++++-----
Mdom/security/nsCSPParser.h | 8+++++++-
Mdom/security/test/sri/test_bug_1364262.html | 12++++++------
Dtesting/web-platform/meta/subresource-integrity/subresource-integrity.html.ini | 6------
5 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/dom/security/SRIMetadata.cpp b/dom/security/SRIMetadata.cpp @@ -8,6 +8,7 @@ #include "hasht.h" #include "mozilla/Logging.h" +#include "nsCSPParser.h" #include "nsICryptoHash.h" static mozilla::LogModule* GetSriMetadataLog() { @@ -38,24 +39,35 @@ SRIMetadata::SRIMetadata(const nsACString& aToken) // split the token into its components mAlgorithm = Substring(aToken, 0, hyphen); uint32_t hashStart = hyphen + 1; + uint32_t hashEnd = aToken.Length(); if (hashStart >= aToken.Length()) { SRIMETADATAERROR(("SRIMetadata::SRIMetadata, invalid (missing digest)")); return; // invalid metadata } + int32_t question = aToken.FindChar('?'); - if (question == -1) { - mHashes.AppendElement( - Substring(aToken, hashStart, aToken.Length() - hashStart)); - } else { + if (question != -1) { MOZ_ASSERT(question > 0); if (static_cast<uint32_t>(question) <= hashStart) { SRIMETADATAERROR( ("SRIMetadata::SRIMetadata, invalid (options w/o digest)")); return; // invalid metadata } - mHashes.AppendElement(Substring(aToken, hashStart, question - hashStart)); + hashEnd = question; } + // The hash must be in valid base64 format as defined by the CSP spec. + // https://w3c.github.io/webappsec-subresource-integrity/#grammardef-hash-expression + // https://w3c.github.io/webappsec-csp/#grammardef-base64-value + if (!nsCSPParser::isValidBase64Value(NS_ConvertUTF8toUTF16( + Substring(aToken, hashStart, hashEnd - hashStart)))) { + SRIMETADATAERROR( + ("SRIMetadata::SRIMetadata, invalid (digest not in base64 format)")); + return; // invalid metadata + } + + mHashes.AppendElement(Substring(aToken, hashStart, hashEnd - hashStart)); + if (mAlgorithm.EqualsLiteral("sha256")) { mAlgorithmType = nsICryptoHash::SHA256; } else if (mAlgorithm.EqualsLiteral("sha384")) { diff --git a/dom/security/nsCSPParser.cpp b/dom/security/nsCSPParser.cpp @@ -91,10 +91,13 @@ bool isGroupDelim(char16_t aSymbol) { aSymbol == '\\' || aSymbol == ']' || aSymbol == '"'); } -static bool isValidBase64Value(const char16_t* cur, const char16_t* end) { +bool nsCSPParser::isValidBase64Value(const nsAString& aValue) { // Using grammar at // https://w3c.github.io/webappsec-csp/#grammardef-nonce-source + const char16_t* cur = aValue.BeginReading(); + const char16_t* end = aValue.EndReading(); + // May end with one or two = if (end > cur && *(end - 1) == EQUALS) end--; if (end > cur && *(end - 1) == EQUALS) end--; @@ -564,8 +567,7 @@ nsCSPNonceSrc* nsCSPParser::nonceSource() { if (dashIndex < 0) { return nullptr; } - if (!isValidBase64Value(expr.BeginReading() + dashIndex + 1, - expr.EndReading())) { + if (!isValidBase64Value(Substring(expr, dashIndex + 1))) { return nullptr; } @@ -594,8 +596,7 @@ nsCSPHashSrc* nsCSPParser::hashSource() { return nullptr; } - if (!isValidBase64Value(expr.BeginReading() + dashIndex + 1, - expr.EndReading())) { + if (!isValidBase64Value(Substring(expr, dashIndex + 1))) { return nullptr; } diff --git a/dom/security/nsCSPParser.h b/dom/security/nsCSPParser.h @@ -41,7 +41,7 @@ const char16_t ATSYMBOL = '@'; class nsCSPParser { public: /** - * The CSP parser only has one publicly accessible function, which is + * The CSP parser only has one main publicly accessible function, which is * parseContentSecurityPolicy. Internally the input string is separated into * string tokens and policy() is called, which starts parsing the policy. The * parser calls one function after the other according the the source-list @@ -56,6 +56,12 @@ class nsCSPParser { bool aDeliveredViaMetaTag, bool aSuppressLogMessages); + /** + * Check if given string is valid base64 as per CSP spec. + * https://w3c.github.io/webappsec-csp/#grammardef-base64-value + */ + static bool isValidBase64Value(const nsAString& aValue); + private: nsCSPParser(policyTokens& aTokens, nsIURI* aSelfURI, nsCSPContext* aCSPContext, bool aDeliveredViaMetaTag, diff --git a/dom/security/test/sri/test_bug_1364262.html b/dom/security/test/sri/test_bug_1364262.html @@ -9,11 +9,11 @@ SimpleTest.waitForExplicitFinish(); SimpleTest.setExpected(["pass", 1]); - function good_correctlyBlockedStylesheet() { - ok(true, "Non-base64 hash blocked the load.") + function bad_incorrectlyBlockedStylesheet() { + ok(false, "Non-base64 hash should not block the load!") }; - function bad_shouldNotLoadStylesheet() { - ok(false, "Non-base64 hashes should not load!") + function good_shouldLoadStylesheet() { + ok(true, "Non-base64 hashes should load") } window.onload = function() { SimpleTest.finish(); @@ -22,8 +22,8 @@ let link = document.createElement('link'); document.head.appendChild(link); link.setAttribute('rel', 'stylesheet'); - link.onerror = good_correctlyBlockedStylesheet; - link.onload = bad_shouldNotLoadStylesheet; + link.onerror = bad_incorrectlyBlockedStylesheet; + link.onload = good_shouldLoadStylesheet; link.integrity = 'sha512-\uD89D\uDF05\uD89D\uDEE6'; link.setAttribute('href', 'data:text/css;small[contenteditable^="false"], summary { }'); </script> diff --git a/testing/web-platform/meta/subresource-integrity/subresource-integrity.html.ini b/testing/web-platform/meta/subresource-integrity/subresource-integrity.html.ini @@ -1,6 +0,0 @@ -[subresource-integrity.html] - [Script: Same-origin with non-Base64 hash.] - expected: FAIL - - [Style: Same-origin with non-Base64 integrity] - expected: FAIL