Merge commit from fork

Fix XSS via `javascript:` url in a link
basecamp · Dec 20, 2024 · 180c8d3 · 180c8d3
2 parents f4d64c2 + f432478
commit 180c8d3
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 2 deletions.
diff --git a/src/test/system/text_formatting_test.js b/src/test/system/text_formatting_test.js
@@ -58,6 +58,18 @@ testGroup("Text formatting", { template: "editor_empty" }, () => {
     expectDocument("ahttp://example.com\n")
   })
 
+  test("inserting a javascript: link is forbidden", async () => {
+    await typeCharacters("XSS")
+    await moveCursor("left")
+    await expandSelection("left")
+    await clickToolbarButton({ attribute: "href" })
+    assert.ok(isToolbarDialogActive({ attribute: "href" }))
+    await typeInToolbarDialog("javascript:alert('XSS')", { attribute: "href" })
+    assert.textAttributes([ 0, 1 ], {})
+    assert.textAttributes([ 1, 2 ], { frozen: true })
+    assert.textAttributes([ 2, 3 ], {})
+  })
+
   test("editing a link", async () => {
     insertString("a")
     const text = Text.textForStringWithAttributes("bc", { href: "http://example.com" })

diff --git a/src/trix/config/toolbar.js b/src/trix/config/toolbar.js
@@ -35,7 +35,7 @@ export default {
     <div class="trix-dialogs" data-trix-dialogs>
       <div class="trix-dialog trix-dialog--link" data-trix-dialog="href" data-trix-dialog-attribute="href">
         <div class="trix-dialog__link-fields">
-          <input type="url" name="href" class="trix-input trix-input--dialog" placeholder="${lang.urlPlaceholder}" aria-label="${lang.url}" required data-trix-input>
+          <input type="url" name="href" class="trix-input trix-input--dialog" placeholder="${lang.urlPlaceholder}" aria-label="${lang.url}" data-trix-validate-href required data-trix-input>
           <div class="trix-button-group">
             <input type="button" class="trix-button trix-button--dialog" value="${lang.link}" data-trix-method="setAttribute">
             <input type="button" class="trix-button trix-button--dialog" value="${lang.unlink}" data-trix-method="removeAttribute">

diff --git a/src/trix/controllers/toolbar_controller.js b/src/trix/controllers/toolbar_controller.js
@@ -2,6 +2,8 @@ import BasicObject from "trix/core/basic_object"
 
 import { findClosestElementFromNode, handleEvent, triggerEvent } from "trix/core/helpers"
 
+import DOMPurify from "dompurify"
+
 const attributeButtonSelector = "[data-trix-attribute]"
 const actionButtonSelector = "[data-trix-action]"
 const toolbarButtonSelector = `${attributeButtonSelector}, ${actionButtonSelector}`
@@ -205,7 +207,10 @@ export default class ToolbarController extends BasicObject {
   setAttribute(dialogElement) {
     const attributeName = getAttributeName(dialogElement)
     const input = getInputForDialog(dialogElement, attributeName)
-    if (input.willValidate && !input.checkValidity()) {
+
+    input.willValidate && input.setCustomValidity("")
+    if (input.willValidate && !input.checkValidity() || !this.safeAttribute(input)) {
+      input.setCustomValidity("Invalid value")
       input.setAttribute("data-trix-validate", "")
       input.classList.add("trix-validate")
       return input.focus()
@@ -215,6 +220,14 @@ export default class ToolbarController extends BasicObject {
     }
   }
 
+  safeAttribute(input) {
+    if (input.hasAttribute("data-trix-validate-href")) {
+      return DOMPurify.isValidAttribute("a", "href", input.value)
+    } else {
+      return true
+    }
+  }
+
   removeAttribute(dialogElement) {
     const attributeName = getAttributeName(dialogElement)
     this.delegate?.toolbarDidRemoveAttribute(attributeName)