From 9a1e3761b42f0f7f49021a185df480dc4aabf423 Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Thu, 12 Dec 2024 15:45:59 -0500
Subject: [PATCH] fix: PermitScrubber accepts frozen tags

Previously, if an invalid/unsafe tag was present, the scrubber
attempted to modify the tags array. Now it properly copies the tags
when they are assigned.

Fixes #195
---
 CHANGELOG.md                | 13 +++++++++++++
 lib/rails/html/scrubbers.rb |  4 ++--
 test/sanitizer_test.rb      |  8 +++++---
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 438e68c..1b25df4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,16 @@
+## next / unreleased
+
+* `PermitScrubber` fully supports frozen "allowed tags".
+
+  v1.6.1 introduced safety checks that may remove unsafe tags from the allowed list, which
+  introduced a regression for applications passing a frozen array of allowed tags. Tags and
+  attributes are now properly copied when they are passed to the scrubber.
+
+  Fixes #195.
+
+  *Mike Dalessio*
+
+
 ## 1.6.1 / 2024-12-02
 
 This is a performance and security release which addresses several possible XSS vulnerabilities.
diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb
index 606b66b..882f961 100644
--- a/lib/rails/html/scrubbers.rb
+++ b/lib/rails/html/scrubbers.rb
@@ -56,11 +56,11 @@ def initialize(prune: false)
       end
 
       def tags=(tags)
-        @tags = validate!(tags, :tags)
+        @tags = validate!(tags.dup, :tags)
       end
 
       def attributes=(attributes)
-        @attributes = validate!(attributes, :attributes)
+        @attributes = validate!(attributes.dup, :attributes)
       end
 
       def scrub(node)
diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb
index 148da0c..f78cd62 100644
--- a/test/sanitizer_test.rb
+++ b/test/sanitizer_test.rb
@@ -1099,7 +1099,7 @@ def test_should_sanitize_across_newlines
     def test_should_prune_mglyph
       # https://hackerone.com/reports/2519936
       input = "<math><mtext><table><mglyph><style><img src=: onerror=alert(1)>"
-      tags = %w(math mtext table mglyph style)
+      tags = %w(math mtext table mglyph style).freeze
 
       actual = nil
       assert_output(nil, /WARNING: 'mglyph' tags cannot be allowed by the PermitScrubber/) do
@@ -1119,7 +1119,7 @@ def test_should_prune_mglyph
     def test_should_prune_malignmark
       # https://hackerone.com/reports/2519936
       input = "<math><mtext><table><malignmark><style><img src=: onerror=alert(1)>"
-      tags = %w(math mtext table malignmark style)
+      tags = %w(math mtext table malignmark style).freeze
 
       actual = nil
       assert_output(nil, /WARNING: 'malignmark' tags cannot be allowed by the PermitScrubber/) do
@@ -1138,7 +1138,9 @@ def test_should_prune_malignmark
 
     def test_should_prune_noscript
       # https://hackerone.com/reports/2509647
-      input, tags = "<div><noscript><p id='</noscript><script>alert(1)</script>'></noscript>", ["p", "div", "noscript"]
+      input = "<div><noscript><p id='</noscript><script>alert(1)</script>'></noscript>"
+      tags = ["p", "div", "noscript"].freeze
+
       actual = nil
       assert_output(nil, /WARNING: 'noscript' tags cannot be allowed by the PermitScrubber/) do
         actual = safe_list_sanitize(input, tags: tags, attributes: %w(id))