Bug 1835157 - Ignore target names which contain both newline and < characters. r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D210480

dom/base/Element.cpp

@@ -3455,7 +3455,23 @@ nsresult Element::PostHandleEventForLinks(EventChainPostVisitor& aVisitor) {
   return rv;
 }
 
-void Element::GetLinkTarget(nsAString& aTarget) { aTarget.Truncate(); }
+// static
+void Element::SanitizeLinkOrFormTarget(nsAString& aTarget) {
+  // <https://html.spec.whatwg.org/multipage/semantics.html#get-an-element's-target>
+  // 2. If target is not null, and contains an ASCII tab or newline and a U+003C
+  // (<), then set target to "_blank".
+  if (!aTarget.IsEmpty() && aTarget.FindCharInSet(u"\t\n\r") != kNotFound &&
+      aTarget.Contains('<')) {
+    aTarget.AssignLiteral("_blank");
+  }
+}
+
+void Element::GetLinkTarget(nsAString& aTarget) {
+  GetLinkTargetImpl(aTarget);
+  SanitizeLinkOrFormTarget(aTarget);
+}
+
+void Element::GetLinkTargetImpl(nsAString& aTarget) { aTarget.Truncate(); }
 
 nsresult Element::CopyInnerTo(Element* aDst, ReparseAttributes aReparse) {
   nsresult rv = aDst->mAttrs.EnsureCapacityToClone(mAttrs);

dom/base/Element.h

@@ -2122,17 +2122,29 @@ class Element : public FragmentOrElement {
    */
   virtual already_AddRefed<nsIURI> GetHrefURI() const { return nullptr; }
 
+  // Step 2. of
+  // <https://html.spec.whatwg.org/multipage/semantics.html#get-an-element's-target>
+  //
+  // Sanitize targets that look like they contain dangling markup.
+  static void SanitizeLinkOrFormTarget(nsAString& aTarget);
+
   /**
+   * <https://html.spec.whatwg.org/multipage/semantics.html#get-an-element's-target>
+   * (Excluding <form>)
+   *
    * Get the target of this link element. Consumers should established that
    * this element is a link (probably using IsLink) before calling this
-   * function (or else why call it?)
+   * function (or else why call it?). This method neuters probably markup
+   * injection attempts.
    *
    * Note: for HTML this gets the value of the 'target' attribute; for XLink
    * this gets the value of the xlink:_moz_target attribute, or failing that,
    * the value of xlink:show, converted to a suitably equivalent named target
    * (e.g. _blank).
    */
-  virtual void GetLinkTarget(nsAString& aTarget);
+  void GetLinkTarget(nsAString& aTarget);
+
+  virtual void GetLinkTargetImpl(nsAString& aTarget);
 
   virtual bool Translate() const;
 

dom/html/HTMLAnchorElement.cpp

@@ -134,7 +134,7 @@ nsresult HTMLAnchorElement::PostHandleEvent(EventChainPostVisitor& aVisitor) {
   return PostHandleEventForAnchors(aVisitor);
 }
 
-void HTMLAnchorElement::GetLinkTarget(nsAString& aTarget) {
+void HTMLAnchorElement::GetLinkTargetImpl(nsAString& aTarget) {
   GetAttr(nsGkAtoms::target, aTarget);
   if (aTarget.IsEmpty()) {
     GetBaseTarget(aTarget);

dom/html/HTMLAnchorElement.h

@@ -55,7 +55,7 @@ class HTMLAnchorElement final : public nsGenericHTMLElement,
   MOZ_CAN_RUN_SCRIPT
   nsresult PostHandleEvent(EventChainPostVisitor& aVisitor) override;
 
-  void GetLinkTarget(nsAString& aTarget) override;
+  void GetLinkTargetImpl(nsAString& aTarget) override;
   already_AddRefed<nsIURI> GetHrefURI() const override;
 
   void BeforeSetAttr(int32_t aNamespaceID, nsAtom* aName,

dom/html/HTMLAreaElement.cpp

@@ -49,7 +49,7 @@ nsresult HTMLAreaElement::PostHandleEvent(EventChainPostVisitor& aVisitor) {
   return PostHandleEventForAnchors(aVisitor);
 }
 
-void HTMLAreaElement::GetLinkTarget(nsAString& aTarget) {
+void HTMLAreaElement::GetLinkTargetImpl(nsAString& aTarget) {
   GetAttr(nsGkAtoms::target, aTarget);
   if (aTarget.IsEmpty()) {
     GetBaseTarget(aTarget);

dom/html/HTMLAreaElement.h

@@ -39,7 +39,7 @@ class HTMLAreaElement final : public nsGenericHTMLElement, public Link {
   MOZ_CAN_RUN_SCRIPT
   nsresult PostHandleEvent(EventChainPostVisitor& aVisitor) override;
 
-  void GetLinkTarget(nsAString& aTarget) override;
+  void GetLinkTargetImpl(nsAString& aTarget) override;
   already_AddRefed<nsIURI> GetHrefURI() const override;
 
   virtual nsresult BindToTree(BindContext&, nsINode& aParent) override;

dom/html/HTMLFormElement.cpp

@@ -1671,6 +1671,22 @@ nsresult HTMLFormElement::GetActionURL(nsIURI** aActionURL,
   return rv;
 }
 
+void HTMLFormElement::GetSubmissionTarget(nsGenericHTMLElement* aSubmitter,
+                                          nsAString& aTarget) {
+  // https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#form-submission-algorithm
+  // 19. If the submitter element is a submit button and it has a formtarget
+  // attribute, then set formTarget to the formtarget attribute value.
+  // 20. Let target be the result of getting an element's target given
+  // submitter's form owner and formTarget.
+  //
+  // Note: Falling back to the base target is part of "get an element's target".
+  if (!(aSubmitter && aSubmitter->GetAttr(nsGkAtoms::formtarget, aTarget)) &&
+      !GetAttr(nsGkAtoms::target, aTarget)) {
+    GetBaseTarget(aTarget);
+  }
+  SanitizeLinkOrFormTarget(aTarget);
+}
+
 nsGenericHTMLFormElement* HTMLFormElement::GetDefaultSubmitElement() const {
   MOZ_ASSERT(mDefaultSubmitElement == mFirstSubmitInElements ||
                  mDefaultSubmitElement == mFirstSubmitNotInElements,

dom/html/HTMLFormElement.h

@@ -481,6 +481,11 @@ class HTMLFormElement final : public nsGenericHTMLElement {
    */
   nsresult GetActionURL(nsIURI** aActionURL, Element* aOriginatingElement);
 
+  // Get the target to submit to. This is either the submitter's |formtarget| or
+  // the form's |target| (Including <base>).
+  void GetSubmissionTarget(nsGenericHTMLElement* aSubmitter,
+                           nsAString& aTarget);
+
   // Returns a number for this form that is unique within its owner document.
   // This is used by nsContentUtils::GenerateStateKey to identify form controls
   // that are inserted into the document by the parser.

dom/html/HTMLFormSubmission.cpp

@@ -828,18 +828,8 @@ nsresult HTMLFormSubmission::GetFromForm(HTMLFormElement* aForm,
   }
 
   // Get target
-  // The target is the submitter element formtarget attribute if the element
-  // is a submit control and has such an attribute.
-  // Otherwise, the target is the form owner's target attribute,
-  // if it has such an attribute.
-  // Finally, if one of the child nodes of the head element is a base element
-  // with a target attribute, then the value of the target attribute of the
-  // first such base element; or, if there is no such element, the empty string.
   nsAutoString target;
-  if (!(aSubmitter && aSubmitter->GetAttr(nsGkAtoms::formtarget, target)) &&
-      !aForm->GetAttr(nsGkAtoms::target, target)) {
-    aForm->GetBaseTarget(target);
-  }
+  aForm->GetSubmissionTarget(aSubmitter, target);
 
   // Get encoding type (default: urlencoded)
   int32_t enctype = NS_FORM_ENCTYPE_URLENCODED;

dom/svg/SVGAElement.cpp

@@ -210,7 +210,7 @@ already_AddRefed<nsIURI> SVGAElement::GetHrefURI() const {
   return nullptr;
 }
 
-void SVGAElement::GetLinkTarget(nsAString& aTarget) {
+void SVGAElement::GetLinkTargetImpl(nsAString& aTarget) {
   mStringAttributes[TARGET].GetAnimValue(aTarget, this);
   if (aTarget.IsEmpty()) {
     static Element::AttrValuesArray sShowVals[] = {nsGkAtoms::_new,

dom/svg/SVGAElement.h

@@ -52,7 +52,7 @@ class SVGAElement final : public SVGAElementBase, public Link {
   int32_t TabIndexDefault() override;
   Focusable IsFocusableWithoutStyle(IsFocusableFlags) override;
 
-  void GetLinkTarget(nsAString& aTarget) override;
+  void GetLinkTargetImpl(nsAString& aTarget) override;
   already_AddRefed<nsIURI> GetHrefURI() const override;
   bool HasHref() const;
 

layout/generic/nsImageFrame.cpp

@@ -1050,9 +1050,8 @@ bool nsImageFrame::ShouldCreateImageFrameForContentProperty(
 // Check if we want to use an image frame or just let the frame constructor make
 // us into an inline, and if so, which kind of image frame should we create.
 /* static */
-auto nsImageFrame::ImageFrameTypeFor(const Element& aElement,
-                                     const ComputedStyle& aStyle)
-    -> ImageFrameType {
+auto nsImageFrame::ImageFrameTypeFor(
+    const Element& aElement, const ComputedStyle& aStyle) -> ImageFrameType {
   if (ShouldCreateImageFrameForContentProperty(aElement, aStyle)) {
     // Prefer the content property, for compat reasons, see bug 1484928.
     return ImageFrameType::ForContentProperty;
@@ -2617,7 +2616,7 @@ bool nsImageFrame::GetAnchorHREFTargetAndNode(nsIURI** aHref, nsString& aTarget,
     }
 
     if (auto* anchor = HTMLAnchorElement::FromNode(content)) {
-      anchor->GetTarget(aTarget);
+      anchor->GetLinkTarget(aTarget);
     }
     NS_ADDREF(*aNode = content);
     return *aHref != nullptr;

testing/web-platform/meta/html/browsers/windows/dangling-markup-window-name.html.ini

@@ -1,15 +0,0 @@
-[dangling-markup-window-name.html]
-  [Dangling Markup with "\\n" in target is reset when set by <a> tag]
-    expected: FAIL
-
-  [Dangling Markup with "\\r" in target is reset when set by <a> tag]
-    expected: FAIL
-
-  [Dangling Markup with "\\t" in target is reset when set by <a> tag]
-    expected: FAIL
-
-  [Dangling Markup in target is reset when set by <form> tag]
-    expected: FAIL
-
-  [Dangling Markup in target is reset when set by <base> tag]
-    expected: FAIL