From 12ee78262ef9a105b548f3c024bc7bbc587af263 Mon Sep 17 00:00:00 2001
From: Jacopo Beschi <jacopobeschi@37signals.com>
Date: Wed, 18 Dec 2024 17:52:52 +0100
Subject: [PATCH 1/2] Fix XSS via `javascript:` url in a link

Prevously, was possible to trigger XSS setting as link an URL like
`javascript:alert('XSS')`.
Fix it via a custom HTML input validation pattern to block both
`javascript:` and `data:` URLs.
---
 src/test/system/text_formatting_test.js | 12 ++++++++++++
 src/trix/config/toolbar.js              |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/test/system/text_formatting_test.js b/src/test/system/text_formatting_test.js
index 0b099c5f0..ce7e775e6 100644
--- 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
index d45c68bee..2d97de337 100644
--- 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}" pattern="^(?!(javascript:|data:)).*" 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">

From f432478fabe7f6763df7f982674b10535bfcf118 Mon Sep 17 00:00:00 2001
From: Jacopo Beschi <jacopobeschi@37signals.com>
Date: Thu, 19 Dec 2024 10:40:51 +0100
Subject: [PATCH 2/2] Switch from JS pattern to DOMPurity.isValidAttribute

This should any cover edge case not covered by the Regexp.
---
 src/trix/config/toolbar.js                 |  2 +-
 src/trix/controllers/toolbar_controller.js | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/trix/config/toolbar.js b/src/trix/config/toolbar.js
index 2d97de337..1eb44fd3f 100644
--- 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}" pattern="^(?!(javascript:|data:)).*" 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
index a1178fea7..7b1b4ebcb 100644
--- 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)