350 lines
17 KiB
Diff
350 lines
17 KiB
Diff
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 | 12 +++++++++++
|
|
.../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 | 21 +++++++++++++++++++
|
|
.../platform/runtime_enabled_features.json5 | 6 ++++++
|
|
14 files changed, 132 insertions(+), 3 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
|
|
@@ -5080,6 +5080,14 @@ void Document::ProcessBaseElement() {
|
|
KURL base_element_url;
|
|
if (href) {
|
|
StringView 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);
|
|
}
|
|
@@ -5097,6 +5105,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();
|
|
}
|
|
}
|
|
|
|
@@ -5129,6 +5145,8 @@ void Document::ProcessBaseElement() {
|
|
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
|
|
@@ -4117,8 +4117,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
|
|
@@ -817,7 +817,8 @@ class CORE_EXPORT Element : public ContainerNode {
|
|
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/renderer/core/dom/cdata_section.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/comment.h"
|
|
#include "third_party/blink/renderer/core/dom/document.h"
|
|
#include "third_party/blink/renderer/core/dom/document_fragment.h"
|
|
@@ -245,6 +247,16 @@ void MarkupFormatter::AppendEndMarkup(StringBuilder& result,
|
|
void MarkupFormatter::AppendAttributeValue(StringBuilder& result,
|
|
const String& attribute,
|
|
bool document_is_html) {
|
|
+ // https://chromium-review.googlesource.com/c/chromium/src/+/6905283
|
|
+ // if (RuntimeEnabledFeatures::LogDanglingAttributesEnabled()) {
|
|
+ // if (attribute.Contains('<') || attribute.Contains('>')) {
|
|
+ // document.AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
|
|
+ // mojom::ConsoleMessageSource::kSecurity,
|
|
+ // mojom::ConsoleMessageLevel::kInfo,
|
|
+ // "Bromite Dangling Markup Prevention: '" + attribute +
|
|
+ // "' is not allowed as parameter value."));
|
|
+ // }
|
|
+ // }
|
|
EntityMask entity_mask = document_is_html ? kEntityMaskInHTMLAttributeValue
|
|
: kEntityMaskInAttributeValue;
|
|
AppendCharactersReplacingEntities(result, attribute, 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
|
|
@@ -547,7 +547,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
|
|
@@ -41,6 +41,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
|
|
@@ -121,6 +121,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(StringToIntLoose(value).value_or(0));
|
|
} 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
|
|
@@ -171,6 +171,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;
|
|
@@ -185,6 +191,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
|
|
@@ -72,6 +72,8 @@ class CORE_EXPORT HTMLIFrameElement : public HTMLFrameElementBase,
|
|
String srcdoc() const;
|
|
void setSrcdoc(const V8UnionStringOrTrustedHTML*, ExceptionState&);
|
|
|
|
+ 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"
|
|
@@ -148,6 +149,14 @@ void FrameLoadRequest::ResolveBlobURLIfNeeded() {
|
|
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
|
|
@@ -22,6 +22,9 @@
|
|
|
|
#include "base/compiler_specific.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"
|
|
@@ -42,6 +45,11 @@ 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)
|
|
@@ -212,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
|
|
@@ -3658,6 +3658,12 @@
|
|
name: "MacDisableCtrlHomeEnd",
|
|
status: "stable",
|
|
},
|
|
+ {
|
|
+ // Enables log of some dangling attributes
|
|
+ // on the javascript console
|
|
+ name: "LogDanglingAttributes",
|
|
+ status: "experimental"
|
|
+ },
|
|
{
|
|
name: "MachineLearningNeuralNetwork",
|
|
// Enabled by webnn::mojom::features::kWebMachineLearningNeuralNetwork.
|
|
--
|