PawletOS/cromite
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

Log dangling attributes in some html elements

Browse Source
This commit is contained in:
Carmelo Messina
2023-07-09 15:59:03 +02:00
parent 41f20e3e4f
commit 33623cd26f
2 changed files with 278 additions and 138 deletions
Download Patch File Download Diff File Expand all files Collapse all files
build/patches/00Log-dangling-attributes-in-some-html-elements.patch
+278
View File
@@ -0,0 +1,278 @@
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
---
chrome/browser/about_flags.cc | 5 ++++-
.../blink/renderer/core/dom/document.cc | 18 +++++++++++++++++
.../blink/renderer/core/dom/element.cc | 20 ++++++++++++++++++-
third_party/blink/renderer/core/dom/element.h | 3 ++-
.../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 ++
.../blink/renderer/core/page/frame_tree.cc | 16 +++++++++++++++
.../platform/runtime_enabled_features.json5 | 10 ++++++++--
11 files changed, 103 insertions(+), 5 deletions(-)
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -4820,7 +4820,10 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kWebShareDescription, kOsWin | kOsCrOS | kOsMac,
FEATURE_VALUE_TYPE(features::kWebShare)},
#endif // BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
-
+ {"enable-log-dangling-attributes",
+ "Log some dangling attributes",
+ "NOTE: log only", kOsAll,
+ FEATURE_VALUE_TYPE(blink::features::kLogDanglingAttributes)},
#if BUILDFLAG(IS_LINUX)
{"ozone-platform-hint", flag_descriptions::kOzonePlatformHintName,
flag_descriptions::kOzonePlatformHintDescription, kOsLinux,
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
@@ -4460,6 +4460,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);
}
@@ -4478,6 +4486,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();
}
}
@@ -4497,6 +4513,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
@@ -2529,8 +2529,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
@@ -568,7 +568,8 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
virtual const QualifiedName& SubResourceAttributeName() 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/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);
+}
+
KURL 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 {
KURL 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
@@ -142,6 +142,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
@@ -167,6 +167,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;
@@ -181,6 +187,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/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"
@@ -239,6 +242,19 @@ FrameTree::FindResult FrameTree::FindOrCreateFrameForNavigation(
LogDanglingMarkupHistogram(current_frame->GetDocument(), name);
}
+ 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
@@ -1453,8 +1453,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",
},
{
// Non-standard API Event.path. Should be replaced by Event.composedPath.
@@ -2151,6 +2151,12 @@
name: "LongAnimationFrameUKM",
status: "stable"
},
+ {
+ // Enables log of some dangling attributes
+ // on the javascript console
+ name: "LogDanglingAttributes",
+ status: "experimental"
+ },
{
name: "MachineLearningCommon",
implied_by: ["MachineLearningModelLoader", "MachineLearningNeuralNetwork"],
--
2.25.1
build/patches/00Remove-dangling-attributes-in-some-html-elements.patch
-138
View File
@@ -1,138 +0,0 @@
From: uazo <uazo@users.noreply.github.com>
Date: Mon, 5 Jun 2023 17:06:03 +0000
Subject: Remove dangling attributes in some html elements
Removes 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.
---
third_party/blink/renderer/core/dom/element.cc | 12 +++++++++++-
third_party/blink/renderer/core/dom/element.h | 3 ++-
.../blink/renderer/core/html/html_base_element.cc | 6 ++++++
.../blink/renderer/core/html/html_base_element.h | 2 ++
.../blink/renderer/core/html/html_iframe_element.cc | 6 ++++++
.../blink/renderer/core/html/html_iframe_element.h | 2 ++
third_party/blink/renderer/core/page/frame_tree.cc | 2 ++
.../renderer/platform/runtime_enabled_features.json5 | 4 ++--
8 files changed, 33 insertions(+), 4 deletions(-)
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
@@ -2529,8 +2529,18 @@ 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('<')) {
+ 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
@@ -568,7 +568,8 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
virtual const QualifiedName& SubResourceAttributeName() 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/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);
+}
+
KURL 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 {
KURL 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_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
@@ -167,6 +167,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;
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/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
@@ -237,6 +237,8 @@ FrameTree::FindResult FrameTree::FindOrCreateFrameForNavigation(
if (ContainsNewLineAndLessThan(name) && IsRequestFromHtml(request) &&
current_frame->GetDocument()) {
LogDanglingMarkupHistogram(current_frame->GetDocument(), name);
+ // if the name contains a \n or <, the search is always deactivated
+ return FindResult(nullptr, false);
}
const KURL& url = request.GetResourceRequest().Url();
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
@@ -1452,8 +1452,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",
},
{
// Non-standard API Event.path. Should be replaced by Event.composedPath.
--
2.25.1