From 26a34b1bac48ea4a702b3ebaecccd7c21f1cf94e Mon Sep 17 00:00:00 2001 From: ch4n3-yoon Date: Tue, 13 Aug 2024 19:35:59 +0900 Subject: [PATCH 1/4] Fix ReDoS vulnerability in PermitScrubber by optimizing regex --- lib/rails/html/scrubbers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb index 6182abb..da15af9 100644 --- a/lib/rails/html/scrubbers.rb +++ b/lib/rails/html/scrubbers.rb @@ -150,7 +150,7 @@ def scrub_attribute(node, attr_node) Loofah::HTML5::Scrub.scrub_attribute_that_allows_local_ref(attr_node) end - if Loofah::HTML5::SafeList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && attr_name == "xlink:href" && attr_node.value =~ /^\s*[^#\s].*/m + if Loofah::HTML5::SafeList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && attr_name == "xlink:href" && attr_node.value =~ /^\s*[^#].*/m attr_node.remove end From f461d2a6478c017b9b8f4479f049e6e0a25a441e Mon Sep 17 00:00:00 2001 From: ch4n3-yoon Date: Tue, 13 Aug 2024 19:36:33 +0900 Subject: [PATCH 2/4] Add linear performance test to verify ReDoS mitigation in PermitScrubber --- test/sanitizer_test.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb index 8cfb523..0234058 100644 --- a/test/sanitizer_test.rb +++ b/test/sanitizer_test.rb @@ -1026,6 +1026,26 @@ def test_should_sanitize_across_newlines assert_equal "", sanitize_css(raw) end + + def test_linear_perfomance_svg + seq = [5000, 10000, 20000, 40000] + times = [] + + seq.each do |n| + payload = "" + elapsed_time = Benchmark.realtime { + safe_list_sanitize(payload) + } + times << elapsed_time + end + + # Manually check for linear performance growth + times.each_cons(2) do |prev_time, next_time| + assert_operator next_time, :<, prev_time * 4, "ReDoS vulnerability detected! Execution time increased too rapidly." + end + end + + protected def safe_list_sanitize(input, options = {}) module_under_test::SafeListSanitizer.new.sanitize(input, options) From e1c15379862a40f1c041ccd2b3ef8d07c883caab Mon Sep 17 00:00:00 2001 From: ch4n3-yoon Date: Tue, 13 Aug 2024 19:47:05 +0900 Subject: [PATCH 3/4] Fix: Add missing require 'benchmark' to scrubbers_test.rb --- test/scrubbers_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/scrubbers_test.rb b/test/scrubbers_test.rb index b0529ea..ed086b2 100644 --- a/test/scrubbers_test.rb +++ b/test/scrubbers_test.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "benchmark" require "minitest/autorun" require "rails-html-sanitizer" From 713241f7ad163a2cf5489212c67a1b85e13f6686 Mon Sep 17 00:00:00 2001 From: ch4n3-yoon Date: Wed, 14 Aug 2024 18:53:01 +0900 Subject: [PATCH 4/4] Revert "Add linear performance test to verify ReDoS mitigation in PermitScrubber" This reverts commit f461d2a6478c017b9b8f4479f049e6e0a25a441e. --- test/sanitizer_test.rb | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb index 0234058..8cfb523 100644 --- a/test/sanitizer_test.rb +++ b/test/sanitizer_test.rb @@ -1026,26 +1026,6 @@ def test_should_sanitize_across_newlines assert_equal "", sanitize_css(raw) end - - def test_linear_perfomance_svg - seq = [5000, 10000, 20000, 40000] - times = [] - - seq.each do |n| - payload = "" - elapsed_time = Benchmark.realtime { - safe_list_sanitize(payload) - } - times << elapsed_time - end - - # Manually check for linear performance growth - times.each_cons(2) do |prev_time, next_time| - assert_operator next_time, :<, prev_time * 4, "ReDoS vulnerability detected! Execution time increased too rapidly." - end - end - - protected def safe_list_sanitize(input, options = {}) module_under_test::SafeListSanitizer.new.sanitize(input, options)