Skip to content

Commit

Permalink
Merge branch 'h1-2503220-nokogiri-serialization' into flavorjones-202…
Browse files Browse the repository at this point in the history
…4-security-fixes

* h1-2503220-nokogiri-serialization:
  dep: bump Nokogiri dependency to address the foreign style issue
  test: Nokogiri's HTML5 "foreign style serialization" issue
  • Loading branch information
flavorjones committed Dec 1, 2024
2 parents 3fd6e65 + b0220b8 commit d7a94c1
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 27 deletions.
4 changes: 0 additions & 4 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
26 changes: 6 additions & 20 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -114,4 +100,4 @@ DEPENDENCIES
rubocop-rails

BUNDLED WITH
2.5.4
2.5.23
8 changes: 5 additions & 3 deletions rails-html-sanitizer.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
38 changes: 38 additions & 0 deletions test/sanitizer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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;)
Expand Down Expand Up @@ -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

0 comments on commit d7a94c1

Please sign in to comment.