Merge branch 'h1-2503220-nokogiri-serialization' into flavorjones-2024-security-fixes

* h1-2503220-nokogiri-serialization:
  dep: bump Nokogiri dependency to address the foreign style issue
  test: Nokogiri's HTML5 "foreign style serialization" issue

Author: flavorjones

Gemfile

@@ -14,7 +14,3 @@ group :rubocop do
   gem "rubocop-performance", require: false
   gem "rubocop-rails", require: false
 end
-
-# specify gem versions for old rubies
-gem "nokogiri", ">= 1.7"
-gem "activesupport", ">= 5"

Gemfile.lock

@@ -3,7 +3,7 @@ PATH
   specs:
     rails-html-sanitizer (1.6.0)
       loofah (~> 2.21)
-      nokogiri (~> 1.14)
+      nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0)
 
 GEM
   remote: https://rubygems.org/
@@ -37,18 +37,10 @@ GEM
     loofah (2.23.1)
       crass (~> 1.0.2)
       nokogiri (>= 1.12.0)
+    mini_portile2 (2.8.8)
     minitest (5.25.2)
-    nokogiri (1.16.7-aarch64-linux)
-      racc (~> 1.4)
-    nokogiri (1.16.7-arm-linux)
-      racc (~> 1.4)
-    nokogiri (1.16.7-arm64-darwin)
-      racc (~> 1.4)
-    nokogiri (1.16.7-x86-linux)
-      racc (~> 1.4)
-    nokogiri (1.16.7-x86_64-darwin)
-      racc (~> 1.4)
-    nokogiri (1.16.7-x86_64-linux)
+    nokogiri (1.16.8)
+      mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
     parallel (1.26.3)
     parser (3.3.6.0)
@@ -94,17 +86,11 @@ GEM
     uri (1.0.2)
 
 PLATFORMS
-  aarch64-linux
-  arm-linux
-  arm64-darwin
-  x86-linux
-  x86_64-darwin
+  ruby
   x86_64-linux
 
 DEPENDENCIES
-  activesupport (>= 5)
   minitest
-  nokogiri (>= 1.7)
   rails-html-sanitizer!
   rake
   rubocop (>= 1.25.1)
@@ -114,4 +100,4 @@ DEPENDENCIES
   rubocop-rails
 
 BUNDLED WITH
-   2.5.4
+   2.5.23

rails-html-sanitizer.gemspec

@@ -26,8 +26,10 @@ Gem::Specification.new do |spec|
   spec.test_files    = Dir["test/**/*"]
   spec.require_paths = ["lib"]
 
-  # NOTE: There's no need to update dependencies for CVEs in minor releases
-  # when users can simply run `bundle update loofah`.
   spec.add_dependency "loofah", "~> 2.21"
-  spec.add_dependency "nokogiri", "~> 1.14"
+
+  # A fix was shipped in nokogiri v1.15.7 and v1.16.8 without which there is a vulnerability in this gem.
+  spec.add_dependency "nokogiri", [">=1.15.7",
+                                   "!=1.16.0", "!=1.16.0.rc1", "!=1.16.1", "!=1.16.2", "!=1.16.3",
+                                   "!=1.16.4", "!=1.16.5", "!=1.16.6", "!=1.16.7"]
 end

test/sanitizer_test.rb

@@ -976,6 +976,34 @@ def test_combination_of_svg_and_style_with_img_payload_2
       assert_includes(acceptable_results, actual)
     end
 
+    def test_combination_of_svg_and_style_with_escaped_img_payload
+      # https://hackerone.com/reports/2503220
+      input, tags = "<svg><style>&lt;img src onerror=alert(1)>", ["svg", "style"]
+      actual = safe_list_sanitize(input, tags: tags)
+      acceptable_results = [
+        # libxml2
+        "<svg><style>&amp;lt;img src onerror=alert(1)&gt;</style></svg>",
+        # libgumbo
+        "<svg><style>&lt;img src onerror=alert(1)&gt;</style></svg>",
+      ]
+
+      assert_includes(acceptable_results, actual)
+    end
+
+    def test_combination_of_math_and_style_with_escaped_img_payload
+      # https://hackerone.com/reports/2503220
+      input, tags = "<math><style>&lt;img src onerror=alert(1)>", ["math", "style"]
+      actual = safe_list_sanitize(input, tags: tags)
+      acceptable_results = [
+        # libxml2
+        "<math><style>&amp;lt;img src onerror=alert(1)&gt;</style></math>",
+        # libgumbo
+        "<math><style>&lt;img src onerror=alert(1)&gt;</style></math>",
+      ]
+
+      assert_includes(acceptable_results, actual)
+    end
+
     def test_should_sanitize_illegal_style_properties
       raw      = %(display:block; position:absolute; left:0; top:0; width:100%; height:100%; z-index:1; background-color:black; background-image:url(http://www.ragingplatypus.com/i/cam-full.jpg); background-x:center; background-y:center; background-repeat:repeat;)
       expected = %(display:block;width:100%;height:100%;background-color:black;background-x:center;background-y:center;)
@@ -1075,5 +1103,15 @@ class HTML4SafeListSanitizerTest < Minitest::Test
   class HTML5SafeListSanitizerTest < Minitest::Test
     @module_under_test = Rails::HTML5
     include SafeListSanitizerTest
+
+    def test_should_not_be_vulnerable_to_nokogiri_foreign_style_serialization_bug
+      # https://hackerone.com/reports/2503220
+      input = "<svg><style>&lt;img src onerror=alert(1)>"
+      result = Rails::HTML5::SafeListSanitizer.new.sanitize(input, tags: ["svg", "style"])
+      browser = Nokogiri::HTML5::Document.parse(result)
+      xss = browser.at_xpath("//img/@onerror")
+
+      assert_nil(xss)
+    end
   end if loofah_html5_support?
 end