Ignore unknown options to subresource integrity

Source/core/frame/SubresourceIntegrity.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/frame/SubresourceIntegrity.h"
 
 #include "core/HTMLNames.h"
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
@@ skipping 22 matching lines @@
     // Check if it's a base64 encoded value. We're pretty loose here, as there's
     // not much risk in it, and it'll make it simpler for developers.
     return isASCIIAlphanumeric(c) || c == '_' || c == '-' || c == '+' || c == '/' || c == '=';
 }
 
 static bool isTypeCharacter(UChar c)
 {
     return isASCIIAlphanumeric(c) || c == '+' || c == '.' || c == '-';
 }
 
+static bool isOptionNameCharacter(UChar c)
+{
+    return isASCIIAlphanumeric(c) || c == '-';
+}
+
+static bool isOptionValueCharacter(UChar c)
+{
+    return isASCIIAlphanumeric(c) || c == '!' || c == '#' || c == '$' || c == '%' || c == '&' || c == '\'' || c == '*' || c == '+' || c == '-'  || c == '.' || c == '^' || c == '_' || c == '`' || c == '|' || c == '~' || c == '/';
+}
+
 static void logErrorToConsole(const String& message, Document& document)
 {
     document.addConsoleMessage(ConsoleMessage::create(SecurityMessageSource, ErrorMessageLevel, message));
 }
 
 static bool DigestsEqual(const DigestValue& digest1, const DigestValue& digest2)
 {
     if (digest1.size() != digest2.size())
         return false;
 
@@ skipping 143 matching lines @@
     if (position == begin || (position != end && *position != '?')) {
         digest = emptyString();
         return false;
     }
 
     // We accept base64url encoding, but normalize to "normal" base64 internally:
     digest = normalizeToBase64(String(begin, position - begin));
     return true;
 }
 
+bool SubresourceIntegrity::isValidMimeTypeValue(const String& value)
+{
+    Vector<UChar> characters;
+    value.appendTo(characters);
+    const UChar* position = characters.data();
+    const UChar* end = characters.end();
 
-// Before:
-//
-// [algorithm]-[hash]     OR      [algorithm]-[hash]?[options]
-//                   ^                              ^         ^
-//        position/end                       position       end
-//
-// After (if successful: if the method returns false, we make no promises and the caller should exit early):
-//
-// [algorithm]-[hash]     OR      [algorithm]-[hash]?[options]
-//                   ^                                        ^
-//        position/end                             position/end
-bool SubresourceIntegrity::parseMimeType(const UChar*& position, const UChar* end, String& type)
-{
-    type = emptyString();
-
-    if (position == end)
-        return true;
-
-    if (!skipToken<UChar>(position, end, "?ct="))
-        return false;
-
-    const UChar* begin = position;
     skipWhile<UChar, isASCIIAlpha>(position, end);
     if (position == end)
         return false;
 
     if (!skipExactly<UChar>(position, end, '/'))
         return false;
 
-    if (position == end)
-        return false;
-
     skipWhile<UChar, isTypeCharacter>(position, end);
     if (position != end)
         return false;
 
-    type = String(begin, position - begin);
     return true;
 }
 
+
+// Before:
+//
+// [algorithm]-[hash]     OR      [algorithm]-[hash]?[name]=[value]     OR      [algorithm]-[hash]?[name]=[value]?[rest of options]
+//                   ^                              ^              ^                              ^                                ^
+//        position/end                       position            end                       position                              end
+//
+// After (if successful: if the method returns false, we make no promises and the caller should exit early):
+//
+// [algorithm]-[hash]     OR      [algorithm]-[hash]?[name]=[value]     OR      [algorithm]-[hash]?[name]=[value]?[rest of options]

[Mike West, 2015/05/11 03:25:28]

This is such a weird syntax.

+//                   ^                                             ^                                            ^                  ^
+//        position/end                                  position/end                                     position                end
+bool SubresourceIntegrity::parseOption(const UChar*& position, const UChar* end, String& name, String& value)
+{
+    name = emptyString();
+    value = emptyString();
+
+    if (position == end)
+        return true;
+
+    if (!skipExactly<UChar>(position, end, '?'))
+        return false;
+
+    const UChar* begin = position;
+    skipWhile<UChar, isOptionNameCharacter>(position, end);
+    if (position == end)
+        return false;
+
+    name = String(begin, position - begin);
+
+    if (!skipExactly<UChar>(position, end, '=') || position == end)
+        return false;
+
+    begin = position;
+    skipWhile<UChar, isOptionValueCharacter>(position, end);
+
+    value = String(begin, position - begin);
+
+    if (position == end || *position == '?')

[Mike West, 2015/05/11 03:25:28]

Would you mind asserting that `position < end` before dereferencing it?

+        return true;
+
+    return false;
+}
+
 SubresourceIntegrity::IntegrityParseResult SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute, WTF::Vector<IntegrityMetadata>& metadataList, Document& document)
 {
     Vector<UChar> characters;
     attribute.stripWhiteSpace().appendTo(characters);
     const UChar* position = characters.data();
     const UChar* end = characters.end();
     const UChar* currentIntegrityEnd;
 
     metadataList.clear();
     bool error = false;
@@ skipping 36 matching lines @@
         ASSERT(parseResult == AlgorithmValid);
 
         if (!parseDigest(position, currentIntegrityEnd, digest)) {
             logErrorToConsole("Error parsing 'integrity' attribute ('" + attribute + "'). The digest must be a valid, base64-encoded value.", document);
             error = true;
             skipUntil<UChar, isASCIISpace>(position, end);
             UseCounter::count(document, UseCounter::SRIElementWithUnparsableIntegrityAttribute);
             continue;
         }
 
-        if (!parseMimeType(position, currentIntegrityEnd, type)) {
-            logErrorToConsole("Error parsing 'integrity' attribute ('" + attribute + "'). The content type could not be parsed.", document);
-            error = true;
-            skipUntil<UChar, isASCIISpace>(position, end);
-            UseCounter::count(document, UseCounter::SRIElementWithUnparsableIntegrityAttribute);
+        bool skipToNextIntegrity = false;
+        while (*position == '?') {
+            String name, value;
+            if (!parseOption(position, currentIntegrityEnd, name, value)) {
+                logErrorToConsole("Error parsing 'integrity' attribute ('" + attribute + "'). The options could not be parsed.", document);
+                skipToNextIntegrity = true;
+                error = true;
+                skipUntil<UChar, isASCIISpace>(position, end);
+                UseCounter::count(document, UseCounter::SRIElementWithUnparsableIntegrityAttribute);
+                break;
+            }
+
+            if (name == "ct") {
+                if (!isValidMimeTypeValue(value)) {
+                    logErrorToConsole("Error parsing 'integrity' attribute ('" + attribute + "'). The content type could not be parsed.", document);
+                    skipToNextIntegrity = true;
+                    error = true;
+                    skipUntil<UChar, isASCIISpace>(position, end);
+                    UseCounter::count(document, UseCounter::SRIElementWithUnparsableIntegrityAttribute);
+                    break;
+                }
+
+                type = value;
+            } else {
+                logErrorToConsole("Ignoring unrecogized 'integrity' attribute option '" + name + "' with value '" + value + "'.", document);
+            }
+        }
+
+        if (skipToNextIntegrity)
             continue;
-        }
 
         IntegrityMetadata integrityMetadata = {
             digest,
             algorithm,
             type
         };
         metadataList.append(integrityMetadata);
     }
 
     if (metadataList.size() == 0 && error)
         return IntegrityParseNoValidResult;
 
     return IntegrityParseValidResult;
 }
 
 } // namespace blink