Log-dangling-attributes-in-some-html-elements.patch

From: uazo <uazo@users.noreply.github.com>
Date: Mon, 5 Jun 2023 17:06:03 +0000
Subject: Log dangling attributes in some html elements

Log for iframes and the base tag all attributes
containing newlines or the less-then sign that can be exploited
to extract or send otherwise inaccessible information.
under enable-log-dangling-attributes about flag

License: GPL-2.0-or-later - https://spdx.org/licenses/GPL-2.0-or-later.html
---
 ...gling-attributes-in-some-html-elements.inc |  8 +++++++
 .../blink/renderer/core/dom/document.cc       | 18 +++++++++++++++
 .../blink/renderer/core/dom/element.cc        | 20 ++++++++++++++++-
 third_party/blink/renderer/core/dom/element.h |  3 ++-
 .../editing/serializers/markup_formatter.cc   |  9 ++++++++
 .../core/html/forms/html_form_element.cc      |  2 +-
 .../renderer/core/html/html_base_element.cc   |  6 +++++
 .../renderer/core/html/html_base_element.h    |  2 ++
 .../core/html/html_frame_element_base.cc      | 10 +++++++++
 .../renderer/core/html/html_iframe_element.cc | 16 ++++++++++++++
 .../renderer/core/html/html_iframe_element.h  |  2 ++
 .../core/loader/frame_load_request.cc         |  9 ++++++++
 .../blink/renderer/core/page/frame_tree.cc    | 22 +++++++++++++++++++
 .../platform/runtime_enabled_features.json5   | 10 +++++++--
 14 files changed, 132 insertions(+), 5 deletions(-)
 create mode 100644 cromite_flags/chrome/browser/about_flags_cc/Log-dangling-attributes-in-some-html-elements.inc

diff --git a/cromite_flags/chrome/browser/about_flags_cc/Log-dangling-attributes-in-some-html-elements.inc b/cromite_flags/chrome/browser/about_flags_cc/Log-dangling-attributes-in-some-html-elements.inc
new file mode 100644
--- /dev/null
+++ b/cromite_flags/chrome/browser/about_flags_cc/Log-dangling-attributes-in-some-html-elements.inc
@@ -0,0 +1,8 @@
+#ifdef FLAG_SECTION
+
+    {"enable-log-dangling-attributes",
+     "Log some dangling attributes",
+     "NOTE: log only", kOsAll,
+     FEATURE_VALUE_TYPE(blink::features::kLogDanglingAttributes)},
+
+#endif
diff --git a/third_party/blink/renderer/core/dom/document.cc b/third_party/blink/renderer/core/dom/document.cc
--- a/third_party/blink/renderer/core/dom/document.cc
+++ b/third_party/blink/renderer/core/dom/document.cc
@@ -4626,6 +4626,14 @@ void Document::ProcessBaseElement() {
   KURL base_element_url;
   if (href) {
     String stripped_href = StripLeadingAndTrailingHTMLSpaces(*href);
+    if (stripped_href.Contains('\n') || stripped_href.Contains('<')) {
+      AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+          mojom::ConsoleMessageSource::kSecurity,
+          mojom::ConsoleMessageLevel::kInfo,
+          "Bromite Dangling Markup Prevention: '" + stripped_href +
+              "' is not allowed as base href value."));
+      //stripped_href = g_empty_atom;
+    }
     if (!stripped_href.empty())
       base_element_url = KURL(FallbackBaseURL(), stripped_href);
   }
@@ -4643,6 +4651,14 @@ void Document::ProcessBaseElement() {
         !GetExecutionContext()->GetSecurityOrigin()->CanRequest(
             base_element_url)) {
       UseCounter::Count(*this, WebFeature::kBaseWithCrossOriginHref);
+      if (RuntimeEnabledFeatures::LogDanglingAttributesEnabled()) {
+        AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+            mojom::ConsoleMessageSource::kSecurity,
+            mojom::ConsoleMessageLevel::kInfo,
+            "Bromite Dangling Markup Prevention: '" + base_element_url.GetString() +
+                "' URL is cross origin and cannot be used as base URLs for a document."));
+      }
+      // base_element_url = BlankURL();
     }
   }
 
@@ -4674,6 +4690,8 @@ void Document::ProcessBaseElement() {
     if (target->Contains('<'))
       UseCounter::Count(*this, WebFeature::kBaseWithOpenBracketInTarget);
     base_target_ = *target;
+    if (target->Contains('\n') || target->Contains('\r') || target->Contains('<'))
+      base_target_ = g_null_atom;
   } else {
     base_target_ = g_null_atom;
   }
diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc
--- a/third_party/blink/renderer/core/dom/element.cc
+++ b/third_party/blink/renderer/core/dom/element.cc
@@ -2969,8 +2969,26 @@ void Element::StripScriptingAttributes(
   attribute_vector.Shrink(destination);
 }
 
+void Element::RemoveDanglingAttributes(
+    Vector<Attribute, kAttributePrealloc>& attribute_vector) {
+  for (auto& attribute : attribute_vector) {
+    auto value = attribute.Value();
+    if (value.Contains('\n') || value.Contains('<')) {
+      if (RuntimeEnabledFeatures::LogDanglingAttributesEnabled()) {
+        GetDocument().AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+            mojom::ConsoleMessageSource::kSecurity,
+            mojom::ConsoleMessageLevel::kWarning,
+            "'" + value + "' is removed from attribute '" +
+              attribute.GetName().ToString() + "' of element '" +
+              tagName() + "' as may contains dangling markup"));
+      }
+      //attribute.SetValue(g_empty_atom);
+    }
+  }
+}
+
 void Element::ParserSetAttributes(
-    const Vector<Attribute, kAttributePrealloc>& attribute_vector) {
+    Vector<Attribute, kAttributePrealloc>& attribute_vector) {
   DCHECK(!isConnected());
   DCHECK(!parentNode());
   DCHECK(!element_data_);
diff --git a/third_party/blink/renderer/core/dom/element.h b/third_party/blink/renderer/core/dom/element.h
--- a/third_party/blink/renderer/core/dom/element.h
+++ b/third_party/blink/renderer/core/dom/element.h
@@ -693,7 +693,8 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
   virtual bool HasLegalLinkAttribute(const QualifiedName&) const;
 
   // Only called by the parser immediately after element construction.
-  void ParserSetAttributes(const Vector<Attribute, kAttributePrealloc>&);
+  virtual void ParserSetAttributes(Vector<Attribute, kAttributePrealloc>&);
+  void RemoveDanglingAttributes(Vector<Attribute, kAttributePrealloc>&);
 
   // Remove attributes that might introduce scripting from the vector leaving
   // the element unchanged.
diff --git a/third_party/blink/renderer/core/editing/serializers/markup_formatter.cc b/third_party/blink/renderer/core/editing/serializers/markup_formatter.cc
--- a/third_party/blink/renderer/core/editing/serializers/markup_formatter.cc
+++ b/third_party/blink/renderer/core/editing/serializers/markup_formatter.cc
@@ -28,6 +28,8 @@
 #include "third_party/blink/renderer/core/editing/serializers/markup_formatter.h"
 
 #include "third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom-shared.h"
+#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
+#include "third_party/blink/renderer/core/inspector/console_message.h"
 #include "third_party/blink/renderer/core/dom/cdata_section.h"
 #include "third_party/blink/renderer/core/dom/comment.h"
 #include "third_party/blink/renderer/core/dom/document.h"
@@ -211,6 +213,13 @@ void MarkupFormatter::AppendAttributeValue(StringBuilder& result,
                                            const Document& document) {
   if (attribute.Contains('<') || attribute.Contains('>')) {
     document.CountUse(mojom::blink::WebFeature::kAttributeValueContainsLtOrGt);
+    if (RuntimeEnabledFeatures::LogDanglingAttributesEnabled()) {
+      document.AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+              mojom::ConsoleMessageSource::kSecurity,
+              mojom::ConsoleMessageLevel::kInfo,
+              "Bromite Dangling Markup Prevention: '" + attribute +
+                  "' is not allowed as parameter value."));
+    }
   }
 
   EntityMask entity_mask =
diff --git a/third_party/blink/renderer/core/html/forms/html_form_element.cc b/third_party/blink/renderer/core/html/forms/html_form_element.cc
--- a/third_party/blink/renderer/core/html/forms/html_form_element.cc
+++ b/third_party/blink/renderer/core/html/forms/html_form_element.cc
@@ -317,7 +317,7 @@ void HTMLFormElement::PrepareForSubmission(
     if (form_control_element && form_control_element->BlocksFormSubmission()) {
       UseCounter::Count(GetDocument(),
                         WebFeature::kFormSubmittedWithUnclosedFormControl);
-      if (RuntimeEnabledFeatures::UnclosedFormControlIsInvalidEnabled()) {
+      if (((true)) || RuntimeEnabledFeatures::UnclosedFormControlIsInvalidEnabled()) {
         String tag_name = To<HTMLFormControlElement>(element)->tagName();
         GetDocument().AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
             mojom::ConsoleMessageSource::kSecurity,
diff --git a/third_party/blink/renderer/core/html/html_base_element.cc b/third_party/blink/renderer/core/html/html_base_element.cc
--- a/third_party/blink/renderer/core/html/html_base_element.cc
+++ b/third_party/blink/renderer/core/html/html_base_element.cc
@@ -61,6 +61,12 @@ bool HTMLBaseElement::IsURLAttribute(const Attribute& attribute) const {
          HTMLElement::IsURLAttribute(attribute);
 }
 
+void HTMLBaseElement::ParserSetAttributes(
+    Vector<Attribute, kAttributePrealloc>& attribute_vector) {
+  Element::RemoveDanglingAttributes(attribute_vector);
+  Element::ParserSetAttributes(attribute_vector);
+}
+
 String HTMLBaseElement::href() const {
   // This does not use the GetURLAttribute function because that will resolve
   // relative to the document's base URL; base elements like this one can be
diff --git a/third_party/blink/renderer/core/html/html_base_element.h b/third_party/blink/renderer/core/html/html_base_element.h
--- a/third_party/blink/renderer/core/html/html_base_element.h
+++ b/third_party/blink/renderer/core/html/html_base_element.h
@@ -37,6 +37,8 @@ class CORE_EXPORT HTMLBaseElement final : public HTMLElement {
   String href() const;
   void setHref(const AtomicString&);
 
+  void ParserSetAttributes(Vector<Attribute, kAttributePrealloc>&) override;
+
  private:
   bool IsURLAttribute(const Attribute&) const override;
   void ParseAttribute(const AttributeModificationParams&) override;
diff --git a/third_party/blink/renderer/core/html/html_frame_element_base.cc b/third_party/blink/renderer/core/html/html_frame_element_base.cc
--- a/third_party/blink/renderer/core/html/html_frame_element_base.cc
+++ b/third_party/blink/renderer/core/html/html_frame_element_base.cc
@@ -119,6 +119,16 @@ void HTMLFrameElementBase::ParseAttribute(
     frame_name_ = value;
   } else if (name == html_names::kNameAttr) {
     frame_name_ = value;
+    if (value.Contains('\n') || value.Contains('<')) {
+      if (RuntimeEnabledFeatures::LogDanglingAttributesEnabled()) {
+        GetDocument().AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+            mojom::ConsoleMessageSource::kSecurity,
+            mojom::ConsoleMessageLevel::kInfo,
+            "Bromite Dangling Markup Prevention: '" + frame_name_ +
+                "' is not allowed as name value."));
+      }
+      //frame_name_ = g_empty_atom;
+    }
   } else if (name == html_names::kMarginwidthAttr) {
     SetMarginWidth(value.ToInt());
   } else if (name == html_names::kMarginheightAttr) {
diff --git a/third_party/blink/renderer/core/html/html_iframe_element.cc b/third_party/blink/renderer/core/html/html_iframe_element.cc
--- a/third_party/blink/renderer/core/html/html_iframe_element.cc
+++ b/third_party/blink/renderer/core/html/html_iframe_element.cc
@@ -161,6 +161,12 @@ void HTMLIFrameElement::CollectStyleForPresentationAttribute(
   }
 }
 
+void HTMLIFrameElement::ParserSetAttributes(
+    Vector<Attribute, kAttributePrealloc>& attribute_vector) {
+  Element::RemoveDanglingAttributes(attribute_vector);
+  Element::ParserSetAttributes(attribute_vector);
+}
+
 void HTMLIFrameElement::ParseAttribute(
     const AttributeModificationParams& params) {
   const QualifiedName& name = params.name;
@@ -175,6 +181,16 @@ void HTMLIFrameElement::ParseAttribute(
     }
     AtomicString old_name = name_;
     name_ = value;
+    if (name_.Contains('\n') || name_.Contains('<')) {
+      if (RuntimeEnabledFeatures::LogDanglingAttributesEnabled()) {
+        GetDocument().AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+            mojom::ConsoleMessageSource::kSecurity,
+            mojom::ConsoleMessageLevel::kInfo,
+            "Bromite Dangling Markup Prevention: '" + name_ +
+                "' is not allowed as name value."));
+      }
+      //name_ = g_empty_atom;
+    }
     if (name_ != old_name) {
       FrameOwnerPropertiesChanged();
       should_call_did_change_attributes = true;
diff --git a/third_party/blink/renderer/core/html/html_iframe_element.h b/third_party/blink/renderer/core/html/html_iframe_element.h
--- a/third_party/blink/renderer/core/html/html_iframe_element.h
+++ b/third_party/blink/renderer/core/html/html_iframe_element.h
@@ -61,6 +61,8 @@ class CORE_EXPORT HTMLIFrameElement : public HTMLFrameElementBase,
 
   bool Credentialless() const override { return credentialless_; }
 
+  void ParserSetAttributes(Vector<Attribute, kAttributePrealloc>&) override;
+
  private:
   void SetCollapsed(bool) override;
 
diff --git a/third_party/blink/renderer/core/loader/frame_load_request.cc b/third_party/blink/renderer/core/loader/frame_load_request.cc
--- a/third_party/blink/renderer/core/loader/frame_load_request.cc
+++ b/third_party/blink/renderer/core/loader/frame_load_request.cc
@@ -11,6 +11,7 @@
 #include "third_party/blink/public/platform/web_url_request.h"
 #include "third_party/blink/renderer/bindings/core/v8/capture_source_location.h"
 #include "third_party/blink/renderer/core/events/current_input_event.h"
+#include "third_party/blink/renderer/core/inspector/console_message.h"
 #include "third_party/blink/renderer/core/fileapi/public_url_manager.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/html/forms/html_form_element.h"
@@ -144,6 +145,14 @@ const LocalFrameToken* FrameLoadRequest::GetInitiatorFrameToken() const {
 const AtomicString& FrameLoadRequest::CleanNavigationTarget(
     const AtomicString& target) const {
   if (ContainsNewLineAndLessThan(target)) {
+    if (RuntimeEnabledFeatures::LogDanglingAttributesEnabled()) {
+      if (origin_window_->GetFrame() && origin_window_->GetFrame()->GetDocument()) {
+            origin_window_->GetFrame()->GetDocument()->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+          mojom::ConsoleMessageSource::kSecurity,
+          mojom::ConsoleMessageLevel::kWarning,
+          "Bromite Dangling Markup Prevention: '" + target + "' is not allowed as navigation target"));
+      }
+    }
     LogDanglingMarkupHistogram(origin_window_, target);
     if (RuntimeEnabledFeatures::RemoveDanglingMarkupInTargetEnabled()) {
       DEFINE_STATIC_LOCAL(const AtomicString, blank, ("_blank"));
diff --git a/third_party/blink/renderer/core/page/frame_tree.cc b/third_party/blink/renderer/core/page/frame_tree.cc
--- a/third_party/blink/renderer/core/page/frame_tree.cc
+++ b/third_party/blink/renderer/core/page/frame_tree.cc
@@ -21,6 +21,9 @@
 #include "third_party/blink/renderer/core/page/frame_tree.h"
 
 #include "third_party/blink/renderer/core/dom/document.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"
+#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
+#include "third_party/blink/renderer/core/inspector/console_message.h"
 #include "third_party/blink/renderer/core/frame/frame_client.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
@@ -41,6 +44,12 @@ namespace {
 
 const unsigned kInvalidChildCount = ~0U;
 
+bool ContainsNewLineAndLessThan(const AtomicString& target) {
+  return (target.Contains('\n') || target.Contains('\r') ||
+          target.Contains('\t')) &&
+         target.Contains('<');
+}
+
 }  // namespace
 
 FrameTree::FrameTree(Frame* this_frame)
@@ -211,6 +220,19 @@ FrameTree::FindResult FrameTree::FindOrCreateFrameForNavigation(
   if (request.GetNavigationPolicy() != kNavigationPolicyCurrentTab)
     return FindResult(current_frame, false);
 
+  if (ContainsNewLineAndLessThan(name)) {
+    // if the name contains a \n or <, the search is always deactivated
+    if (RuntimeEnabledFeatures::LogDanglingAttributesEnabled()) {
+      if (current_frame->GetDocument()) {
+        current_frame->GetDocument()->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+          mojom::ConsoleMessageSource::kSecurity,
+          mojom::ConsoleMessageLevel::kWarning,
+          "Bromite Dangling Markup Prevention: '" + name.GetString() + "' is not allowed as frame name destination"));
+      }
+      // return FindResult(nullptr, false);
+    }
+  }
+
   const KURL& url = request.GetResourceRequest().Url();
   Frame* frame = FindFrameForNavigationInternal(name, url, &request);
   bool new_window = false;
diff --git a/third_party/blink/renderer/platform/runtime_enabled_features.json5 b/third_party/blink/renderer/platform/runtime_enabled_features.json5
--- a/third_party/blink/renderer/platform/runtime_enabled_features.json5
+++ b/third_party/blink/renderer/platform/runtime_enabled_features.json5
@@ -1690,8 +1690,8 @@
       // Experiment with preventing some instances of mutation XSS
       // by escaping "<" and ">" in attribute values.
       // See: crbug.com/1175016
-      name: "EscapeLtGtInAttributes",
-      status: "experimental",
+      name: "EscapeLtGtInAttributes",  // enabled by default
+      status: "stable",
     },
     {
       name: "EventTimingHandleKeyboardEventSimulatedClick",
@@ -2634,6 +2634,12 @@
       name: "LongTaskFromLongAnimationFrame",
       status: "test",
     },
+    {
+      // Enables log of some dangling attributes
+      // on the javascript console
+      name: "LogDanglingAttributes",
+      status: "experimental"
+    },
     {
       name: "MachineLearningNeuralNetwork",
       // Enabled by webnn::mojom::features::kWebMachineLearningNeuralNetwork.
--