From 16251735e36ebdc302e2f90f2a39cad56879414f Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Sat, 30 Nov 2024 16:16:29 -0500
Subject: [PATCH] fix: disallow 'noscript' from safe lists

https://hackerone.com/reports/2509647
---
 lib/rails/html/scrubbers.rb |  6 ++++++
 test/sanitizer_test.rb      | 35 +++++++++++++++++++++++++++++++++++
 test/scrubbers_test.rb      |  8 ++++++++
 3 files changed, 49 insertions(+)

diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb
index 6182abb..7dde73a 100644
--- a/lib/rails/html/scrubbers.rb
+++ b/lib/rails/html/scrubbers.rb
@@ -134,6 +134,12 @@ def validate!(var, name)
           if var && !var.is_a?(Enumerable)
             raise ArgumentError, "You should pass :#{name} as an Enumerable"
           end
+
+          if var && name == :tags && var.include?("noscript")
+            warn("WARNING: 'noscript' tags cannot be allowed by the PermitScrubber and will be scrubbed")
+            var.delete("noscript")
+          end
+
           var
         end
 
diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb
index 8cfb523..05c4130 100644
--- a/test/sanitizer_test.rb
+++ b/test/sanitizer_test.rb
@@ -1026,6 +1026,24 @@ def test_should_sanitize_across_newlines
       assert_equal "", sanitize_css(raw)
     end
 
+    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"]
+      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))
+      end
+
+      acceptable_results = [
+        # libxml2
+        "<div><p id=\"&lt;/noscript&gt;&lt;script&gt;alert(1)&lt;/script&gt;\"></p></div>",
+        # libgumbo
+        "<div><p id=\"</noscript><script>alert(1)</script>\"></p></div>",
+      ]
+
+      assert_includes(acceptable_results, actual)
+    end
+
     protected
       def safe_list_sanitize(input, options = {})
         module_under_test::SafeListSanitizer.new.sanitize(input, options)
@@ -1075,5 +1093,22 @@ class HTML4SafeListSanitizerTest < Minitest::Test
   class HTML5SafeListSanitizerTest < Minitest::Test
     @module_under_test = Rails::HTML5
     include SafeListSanitizerTest
+
+    def test_should_not_be_vulnerable_to_noscript_attacks
+      # https://hackerone.com/reports/2509647
+      skip("browser assertion requires parse_noscript_content_as_text") unless Nokogiri::VERSION >= "1.17"
+
+      input = '<noscript><p id="</noscript><script>alert(1)</script>"></noscript>'
+
+      result = nil
+      assert_output(nil, /WARNING/) do
+        result = Rails::HTML5::SafeListSanitizer.new.sanitize(input, tags: %w(p div noscript), attributes: %w(id class style))
+      end
+
+      browser = Nokogiri::HTML5::Document.parse(result, parse_noscript_content_as_text: true)
+      xss = browser.at_xpath("//script")
+
+      assert_nil(xss)
+    end
   end if loofah_html5_support?
 end
diff --git a/test/scrubbers_test.rb b/test/scrubbers_test.rb
index b0529ea..05f00a4 100644
--- a/test/scrubbers_test.rb
+++ b/test/scrubbers_test.rb
@@ -121,6 +121,14 @@ def test_leaves_only_supplied_tags_and_attributes
     assert_scrubbed html, '<tag></tag><tag cooler=""></tag>'
   end
 
+  def test_does_not_allow_safelisted_noscript
+    # https://hackerone.com/reports/2509647
+    assert_output(nil, /WARNING: 'noscript' tags cannot be allowed by the PermitScrubber/) do
+      @scrubber.tags = ["div", "noscript", "span"]
+    end
+    assert_equal(["div", "span"], @scrubber.tags)
+  end
+
   def test_leaves_text
     assert_scrubbed("some text")
   end