Commit 7b52a16c authored by jsbell's avatar jsbell Committed by Commit bot

CSP: Don't perform NFC normalization prior to hashing

Normalization is lossy and not called for in the spec. Don't do it.

BUG=487510,545678
R=jww@chromium.org

Review URL: https://codereview.chromium.org/1420483005

Cr-Commit-Position: refs/heads/master@{#357132}
parent 00f44574
ALERT: PASS ALERT: PASS (1/1)
ALERT: PASS CONSOLE ERROR: line 20: Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'sha1-zv73epHrGLk/k/onuSBPoZAxzaA=' 'sha256-6VVrnAGI98OnlK9Y20hAMwfwBE8c8FOtE/jDYM7tPFk='". Either the 'unsafe-inline' keyword, a hash ('sha256-1YpMZRdgC0WhwwFBK0bksRyUnuhzlCJp0nKmbZYUi+Q='), or a nonce ('nonce-...') is required to enable inline execution.
This tests Unicode normalization. While appearing the same, the strings in the scripts are different Unicode points, but through normalization, should be the same when the hash is taken.
This tests Unicode normalization. While appearing the same, the strings in the scripts are different Unicode points. Unicode NFC normalization would make both match the hash, but normalization should not be performed, and so the second script should not run.
...@@ -2,30 +2,32 @@ ...@@ -2,30 +2,32 @@
<html> <html>
<head> <head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta http-equiv="Content-Security-Policy" content="script-src 'sha1-zv73epHrGLk/k/onuSBPoZAxzaA=' 'sha1-gbGNUiHncUNJ+diPbIoc+x6KrLo='"> <meta http-equiv="Content-Security-Policy" content="script-src 'sha1-zv73epHrGLk/k/onuSBPoZAxzaA=' 'sha256-6VVrnAGI98OnlK9Y20hAMwfwBE8c8FOtE/jDYM7tPFk='">
<script> <script>
if (window.testRunner) if (window.testRunner)
testRunner.dumpAsText(); testRunner.dumpAsText();
</script> </script>
<!-- The following two scripts contain two separate code points (U+00C5 <!-- The following two scripts contain two separate code points (U+00C5
and U+212B, respectively) which, depending on your text editor, might be and U+212B, respectively) which, depending on your text editor, might be
rendered the same. However, their difference is important as they should rendered the same. However, their difference is important as they would
be NFC normalized to the same code point, thus they should hash to the be NFC normalized to the same code point, matching the hash. Since NFC
same value.--> normalization should not be performed, the second script should not
<script> match the hash and must not be executed. -->
<script data-alert="PASS (1/1)">
'Å'; 'Å';
alert('PASS'); alert(document.currentScript.dataset.alert);
</script> </script>
<script> <script data-alert="FAIL">
''; '';
alert('PASS'); alert(document.currentScript.dataset.alert);
</script> </script>
</head> </head>
<body> <body>
<p> <p>
This tests Unicode normalization. While appearing the same, the This tests Unicode normalization. While appearing the same, the
strings in the scripts are different Unicode points, but through strings in the scripts are different Unicode points. Unicode NFC
normalization, should be the same when the hash is taken. normalization would make both match the hash, but normalization
should not be performed, and so the second script should not run.
</p> </p>
</body> </body>
</html> </html>
...@@ -26,8 +26,8 @@ namespace { ...@@ -26,8 +26,8 @@ namespace {
String getSha256String(const String& content) String getSha256String(const String& content)
{ {
DigestValue digest; DigestValue digest;
StringUTF8Adaptor normalizedContent = normalizeSource(content); StringUTF8Adaptor utf8Content(content);
bool digestSuccess = computeDigest(HashAlgorithmSha256, normalizedContent.data(), normalizedContent.length(), digest); bool digestSuccess = computeDigest(HashAlgorithmSha256, utf8Content.data(), utf8Content.length(), digest);
if (!digestSuccess) { if (!digestSuccess) {
return "sha256-..."; return "sha256-...";
} }
......
...@@ -430,12 +430,12 @@ bool checkDigest(const String& source, uint8_t hashAlgorithmsUsed, const CSPDire ...@@ -430,12 +430,12 @@ bool checkDigest(const String& source, uint8_t hashAlgorithmsUsed, const CSPDire
if (hashAlgorithmsUsed == ContentSecurityPolicyHashAlgorithmNone) if (hashAlgorithmsUsed == ContentSecurityPolicyHashAlgorithmNone)
return false; return false;
StringUTF8Adaptor normalizedSource = normalizeSource(source); StringUTF8Adaptor utf8Source(source);
for (const auto& algorithmMap : kAlgorithmMap) { for (const auto& algorithmMap : kAlgorithmMap) {
DigestValue digest; DigestValue digest;
if (algorithmMap.cspHashAlgorithm & hashAlgorithmsUsed) { if (algorithmMap.cspHashAlgorithm & hashAlgorithmsUsed) {
bool digestSuccess = computeDigest(algorithmMap.algorithm, normalizedSource.data(), normalizedSource.length(), digest); bool digestSuccess = computeDigest(algorithmMap.algorithm, utf8Source.data(), utf8Source.length(), digest);
if (digestSuccess && isAllowedByAllWithHash<allowed>(policies, CSPHashValue(algorithmMap.cspHashAlgorithm, digest))) if (digestSuccess && isAllowedByAllWithHash<allowed>(policies, CSPHashValue(algorithmMap.cspHashAlgorithm, digest)))
return true; return true;
} }
......
...@@ -67,9 +67,4 @@ bool isMediaTypeCharacter(UChar c) ...@@ -67,9 +67,4 @@ bool isMediaTypeCharacter(UChar c)
return !isASCIISpace(c) && c != '/'; return !isASCIISpace(c) && c != '/';
} }
WTF::StringUTF8Adaptor normalizeSource(const String& source)
{
return WTF::StringUTF8Adaptor(source, WTF::StringUTF8Adaptor::Normalize, WTF::EntitiesForUnencodables);
}
} // namespace } // namespace
...@@ -52,9 +52,6 @@ PLATFORM_EXPORT bool isMediaTypeCharacter(UChar); ...@@ -52,9 +52,6 @@ PLATFORM_EXPORT bool isMediaTypeCharacter(UChar);
// positional and may only appear at the end of a Base64 encoded string. // positional and may only appear at the end of a Base64 encoded string.
PLATFORM_EXPORT bool isBase64EncodedCharacter(UChar); PLATFORM_EXPORT bool isBase64EncodedCharacter(UChar);
// Normalize script or style source for script hash use.
PLATFORM_EXPORT WTF::StringUTF8Adaptor normalizeSource(const String& source);
} // namespace blink } // namespace blink
#endif #endif
Markdown is supported
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