From 19cb047c618b3c25d88deccfd88d1fe09c1dd095 Mon Sep 17 00:00:00 2001
From: Andrew Paseltiner <[email protected]>
Date: Fri, 01 May 2026 12:31:49 -0700
Subject: [PATCH] Surgically fix mixed-case attribute bypass in FrameSerializer

This change adds a case-insensitive scripting attribute check in
FrameSerializer::WillProcessAttribute. This prevents bypasses where
scripting attributes are injected into the DOM with mixed-case (e.g.,
ONLOAD) via namespace-aware APIs like setAttributeNS. Such attributes
bypass the current case-sensitive filter but are activated when the
MHTML is reopened and parsed by the HTML parser (which lowercases all
attribute names).

The check is applied to all attributes regardless of namespace. This is
necessary because the HTML parser is namespace-unaware and will
lowercase any attribute name it encounters during re-parsing of the
MHTML archive, potentially turning a namespaced mixed-case attribute
into a functional event handler.

TODOs have been added to the code to investigate other case-sensitive
attribute checks in the same method (e.g., integrity, srcdoc, and
declarative shadow DOM attributes) that might also be bypassable via the
setAttributeNS vector.

This is a 'best-effort' sanitization approach during serialization.
While it addresses the reported bypass, it does not guarantee absolute
safety against all possible obfuscations. However, it provides a
critical layer of defense-in-depth, complementing the sandboxing applied
when MHTML documents are loaded with scripts enabled.

A regression test is included to verify the fix across various casing
and namespace scenarios.

Fixed: 503865896
Change-Id: Ib040014f460b7144c4826df0abc7bd4578a85855
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7781114
Reviewed-by: Ian Kilpatrick <[email protected]>
Commit-Queue: Andrew Paseltiner <[email protected]>
Reviewed-by: Łukasz Anforowicz <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1623999}
---

diff --git a/third_party/blink/renderer/core/frame/frame_serializer.cc b/third_party/blink/renderer/core/frame/frame_serializer.cc
index 8190a9b6..81b86a5d 100644
--- a/third_party/blink/renderer/core/frame/frame_serializer.cc
+++ b/third_party/blink/renderer/core/frame/frame_serializer.cc
@@ -532,6 +532,8 @@
     // The special attribute in a template element to denote the shadow DOM
     // should only be generated from MHTML serialization. If it is found in the
     // original page, it should be ignored.
+    // TODO(crbug.com/503865896): These checks are case-sensitive and might be
+    // bypassed by mixed-case attributes created via setAttributeNS.
     if (IsA<HTMLTemplateElement>(element) &&
         (attribute.LocalName() == kShadowModeAttributeName ||
          attribute.LocalName() == kShadowDelegatesFocusAttributeName) &&
@@ -542,6 +544,8 @@
     // If srcdoc attribute for frame elements will be rewritten as src attribute
     // containing link instead of html contents, don't ignore the attribute.
     // Bail out now to avoid the check in Element::isScriptingAttribute.
+    // TODO(crbug.com/503865896): This check is case-sensitive and might be
+    // bypassed by mixed-case attributes created via setAttributeNS.
     bool is_src_doc_attribute = IsA<HTMLFrameElementBase>(element) &&
                                 attribute.GetName() == html_names::kSrcdocAttr;
     String new_link_for_the_element;
@@ -551,6 +555,8 @@
     }
 
     //  Drop integrity attribute for those links with subresource loaded.
+    // TODO(crbug.com/503865896): This check is case-sensitive and might be
+    // bypassed by mixed-case attributes created via setAttributeNS.
     auto* html_link_element = DynamicTo<HTMLLinkElement>(element);
     if (attribute.LocalName() == html_names::kIntegrityAttr &&
         html_link_element && html_link_element->sheet()) {
@@ -562,6 +568,35 @@
     if (element.IsScriptingAttribute(attribute)) {
       return EmitAttributeChoice::kIgnore;
     }
+
+    // Check if the attribute is a scripting attribute in a case-insensitive
+    // way. While attributes are usually lowercased by the HTML parser, they
+    // can be created with mixed-case via DOM APIs like setAttributeNS.
+    // When saved to MHTML and later reopened, the HTML parser will lowercase
+    // them, potentially activating an event handler that was bypassed during
+    // serialization.
+    //
+    // We perform this check for all attributes, regardless of namespace,
+    // because the HTML parser is namespace-unaware and will lowercase any
+    // attribute name it encounters. Attributes with non-null namespaces
+    // (that aren't well-known to the HTML serializer) are emitted as regular
+    // attributes that the parser will then treat as potential event handlers.
+    // See crbug.com/503865896.
+    AtomicString lower_name = attribute.LocalName().ToAsciiLower();
+    if (lower_name != attribute.LocalName()) {
+      // We use g_null_atom for the prefix and namespace to simulate how the
+      // attribute will be treated by a namespace-unaware HTML parser upon
+      // re-parsing. This ensures that IsScriptingAttribute correctly identifies
+      // event handlers (which must be in the null namespace) and URL
+      // attributes (which are compared against null-namespaced QualifiedNames).
+      Attribute lower_attribute(
+          QualifiedName(g_null_atom, lower_name, g_null_atom),
+          attribute.Value());
+      if (element.IsScriptingAttribute(lower_attribute)) {
+        return EmitAttributeChoice::kIgnore;
+      }
+    }
+
     return EmitAttributeChoice::kEmit;
   }
 
diff --git a/third_party/blink/renderer/core/frame/frame_serializer_test.cc b/third_party/blink/renderer/core/frame/frame_serializer_test.cc
index 06f0285..fb6a1ae 100644
--- a/third_party/blink/renderer/core/frame/frame_serializer_test.cc
+++ b/third_party/blink/renderer/core/frame/frame_serializer_test.cc
@@ -49,6 +49,7 @@
 #include "third_party/blink/renderer/core/exported/web_view_impl.h"
 #include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
 #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
+#include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/heap/thread_state.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_error.h"
 #include "third_party/blink/renderer/platform/mhtml/serialized_resource.h"
@@ -210,7 +211,11 @@
 
   test::TaskEnvironment task_environment_;
   ScopedTestingPlatformSupport<TestingPlatformSupport> platform_;
+
+ protected:
   frame_test_helpers::WebViewHelper helper_;
+
+ private:
   std::string folder_;
   KURL base_url_;
   Deque<SerializedResource> resources_;
@@ -585,6 +590,72 @@
   EXPECT_FALSE(data.contains("onclick"));
 }
 
+// Regression test for crbug.com/503865896
+TEST_F(FrameSerializerTest, MixedCaseScriptingAttributesStripped) {
+  SetBaseFolder("frameserializer/elements/");
+
+  RegisterURL("empty.html", "empty.txt", "text/html");
+  Serialize("empty.html");
+
+  // Inject mixed-case scripting attributes.
+  Element* body = helper_.LocalMainFrame()->GetFrame()->GetDocument()->body();
+  body->setAttributeNS(g_null_atom, AtomicString("ONLOAD"),
+                       AtomicString("alert(1)"), IGNORE_EXCEPTION_FOR_TESTING);
+  body->setAttributeNS(g_null_atom, AtomicString("oNCLICK"),
+                       AtomicString("alert(2)"), IGNORE_EXCEPTION_FOR_TESTING);
+
+  Element* anchor =
+      helper_.LocalMainFrame()->GetFrame()->GetDocument()->CreateRawElement(
+          html_names::kATag);
+  anchor->setAttributeNS(g_null_atom, AtomicString("HREF"),
+                         AtomicString("javascript:alert(3)"),
+                         IGNORE_EXCEPTION_FOR_TESTING);
+  body->AppendChild(anchor);
+
+  Element* iframe =
+      helper_.LocalMainFrame()->GetFrame()->GetDocument()->CreateRawElement(
+          html_names::kIFrameTag);
+  iframe->setAttributeNS(g_null_atom, AtomicString("SRCDOC"),
+                         AtomicString("<html></html>"),
+                         IGNORE_EXCEPTION_FOR_TESTING);
+  body->AppendChild(iframe);
+
+  // Inject a mixed-case attribute with a non-null namespace.
+  // This should also be stripped because the HTML parser (which is
+  // namespace-unaware) will lowercase it and activate it as an event handler
+  // upon reload. See crbug.com/503865896.
+  body->setAttributeNS(AtomicString("http://example.com"),
+                       AtomicString("ONLOAD"), AtomicString("alert(4)"),
+                       IGNORE_EXCEPTION_FOR_TESTING);
+
+  // Re-serialize the same frame.
+  GetResources().clear();
+  base::RunLoop run_loop;
+  FrameSerializer::SerializeFrame(
+      *this, *helper_.LocalMainFrame()->GetFrame(),
+      base::BindLambdaForTesting([&](Deque<SerializedResource> resources) {
+        for (auto& res : resources) {
+          GetResources().push_back(res);
+        }
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+
+  String data = GetSerializedData("empty.html", "text/html");
+  EXPECT_EQ(data.FindIgnoringAsciiCase("onload"), kNotFound);
+  EXPECT_EQ(data.FindIgnoringAsciiCase("onclick"), kNotFound);
+  EXPECT_EQ(data.FindIgnoringAsciiCase("href"), kNotFound);
+  EXPECT_EQ(data.FindIgnoringAsciiCase("srcdoc"), kNotFound);
+
+  // Even the attribute with a non-null namespace should be stripped if its
+  // lowercased name matches a scripting attribute, because the HTML parser
+  // will activate it.
+  EXPECT_EQ(data.FindIgnoringAsciiCase("alert(4)"), kNotFound);
+
+  // Ensure that *something* was returned.

diff --git a/third_party/blink/renderer/core/frame/frame_serializer_test.cc b/third_party/blink/renderer/core/frame/frame_serializer_test.cc
index 06f0285..fb6a1ae 100644
--- a/third_party/blink/renderer/core/frame/frame_serializer_test.cc
+++ b/third_party/blink/renderer/core/frame/frame_serializer_test.cc
@@ -49,6 +49,7 @@
 #include "third_party/blink/renderer/core/exported/web_view_impl.h"
 #include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
 #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
+#include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/heap/thread_state.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_error.h"
 #include "third_party/blink/renderer/platform/mhtml/serialized_resource.h"
@@ -210,7 +211,11 @@
 
   test::TaskEnvironment task_environment_;
   ScopedTestingPlatformSupport<TestingPlatformSupport> platform_;
+
+ protected:
   frame_test_helpers::WebViewHelper helper_;
+
+ private:
   std::string folder_;
   KURL base_url_;
   Deque<SerializedResource> resources_;
@@ -585,6 +590,72 @@
   EXPECT_FALSE(data.contains("onclick"));
 }
 
+// Regression test for crbug.com/503865896
+TEST_F(FrameSerializerTest, MixedCaseScriptingAttributesStripped) {
+  SetBaseFolder("frameserializer/elements/");
+
+  RegisterURL("empty.html", "empty.txt", "text/html");
+  Serialize("empty.html");
+
+  // Inject mixed-case scripting attributes.
+  Element* body = helper_.LocalMainFrame()->GetFrame()->GetDocument()->body();
+  body->setAttributeNS(g_null_atom, AtomicString("ONLOAD"),
+                       AtomicString("alert(1)"), IGNORE_EXCEPTION_FOR_TESTING);
+  body->setAttributeNS(g_null_atom, AtomicString("oNCLICK"),
+                       AtomicString("alert(2)"), IGNORE_EXCEPTION_FOR_TESTING);
+
+  Element* anchor =
+      helper_.LocalMainFrame()->GetFrame()->GetDocument()->CreateRawElement(
+          html_names::kATag);
+  anchor->setAttributeNS(g_null_atom, AtomicString("HREF"),
+                         AtomicString("javascript:alert(3)"),
+                         IGNORE_EXCEPTION_FOR_TESTING);
+  body->AppendChild(anchor);
+
+  Element* iframe =
+      helper_.LocalMainFrame()->GetFrame()->GetDocument()->CreateRawElement(
+          html_names::kIFrameTag);
+  iframe->setAttributeNS(g_null_atom, AtomicString("SRCDOC"),
+                         AtomicString("<html></html>"),
+                         IGNORE_EXCEPTION_FOR_TESTING);
+  body->AppendChild(iframe);
+
+  // Inject a mixed-case attribute with a non-null namespace.
+  // This should also be stripped because the HTML parser (which is
+  // namespace-unaware) will lowercase it and activate it as an event handler
+  // upon reload. See crbug.com/503865896.
+  body->setAttributeNS(AtomicString("http://example.com"),
+                       AtomicString("ONLOAD"), AtomicString("alert(4)"),
+                       IGNORE_EXCEPTION_FOR_TESTING);
+
+  // Re-serialize the same frame.
+  GetResources().clear();
+  base::RunLoop run_loop;
+  FrameSerializer::SerializeFrame(
+      *this, *helper_.LocalMainFrame()->GetFrame(),
+      base::BindLambdaForTesting([&](Deque<SerializedResource> resources) {
+        for (auto& res : resources) {
+          GetResources().push_back(res);
+        }
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+
+  String data = GetSerializedData("empty.html", "text/html");
+  EXPECT_EQ(data.FindIgnoringAsciiCase("onload"), kNotFound);
+  EXPECT_EQ(data.FindIgnoringAsciiCase("onclick"), kNotFound);
+  EXPECT_EQ(data.FindIgnoringAsciiCase("href"), kNotFound);
+  EXPECT_EQ(data.FindIgnoringAsciiCase("srcdoc"), kNotFound);
+
+  // Even the attribute with a non-null namespace should be stripped if its
+  // lowercased name matches a scripting attribute, because the HTML parser
+  // will activate it.
+  EXPECT_EQ(data.FindIgnoringAsciiCase("alert(4)"), kNotFound);
+
+  // Ensure that *something* was returned.
+  EXPECT_NE(data.FindIgnoringAsciiCase("<a"), kNotFound);
+}
+
 TEST_F(FrameSerializerTest, DontIncludeErrorImage) {
   SetBaseFolder("frameserializer/image/");