Skip to content

Commit 74dcb80

Browse files
committed
Properly restrict SVG href attributes
PermitScrubber reimplements Loofah's SVG local-reference restriction, which covered only `xlink:href`. SVG 2 also allows plain `href`, so `<use href="https://example.com/x.svg#p"/>` passed through intact. Restrict both via Loofah's SVG_HREF_ATTRIBUTES set (Loofah >= 2.25.2), mirroring the related Loofah fix for GHSA-9wjq-cp2p-hrgf. ref: GHSA-cj75-f6xr-r4g7
1 parent 11ee440 commit 74dcb80

4 files changed

Lines changed: 45 additions & 7 deletions

File tree

Gemfile.lock

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ PATH
22
remote: .
33
specs:
44
rails-html-sanitizer (1.7.0)
5-
loofah (~> 2.25)
5+
loofah (~> 2.25, >= 2.25.2)
66
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)
77

88
GEM
@@ -26,15 +26,15 @@ GEM
2626
bigdecimal (4.1.2)
2727
concurrent-ruby (1.3.7)
2828
connection_pool (3.0.2)
29-
crass (1.0.6)
29+
crass (1.0.7)
3030
drb (2.2.3)
3131
i18n (1.14.8)
3232
concurrent-ruby (~> 1.0)
3333
json (2.19.8)
3434
language_server-protocol (3.17.0.5)
3535
lint_roller (1.1.0)
3636
logger (1.7.0)
37-
loofah (2.25.1)
37+
loofah (2.25.2)
3838
crass (~> 1.0.2)
3939
nokogiri (>= 1.12.0)
4040
minitest (5.27.0)
@@ -132,14 +132,14 @@ CHECKSUMS
132132
bundler (4.0.13) sha256=19f08be7f27022cf0b89f27da0b044ae075e8270a9ef44ad248a932614e1ca3b
133133
concurrent-ruby (1.3.7) sha256=4412caec3a5ea2e5fdc52076724c071a81f2c0593d83b2ac8cbb8ca63b3151b0
134134
connection_pool (3.0.2) sha256=33fff5ba71a12d2aa26cb72b1db8bba2a1a01823559fb01d29eb74c286e62e0a
135-
crass (1.0.6) sha256=dc516022a56e7b3b156099abc81b6d2b08ea1ed12676ac7a5657617f012bd45d
135+
crass (1.0.7) sha256=94868719948664c89ddcaf0a37c65048413dfcb1c869470a5f7a7ceb5390b295
136136
drb (2.2.3) sha256=0b00d6fdb50995fe4a45dea13663493c841112e4068656854646f418fda13373
137137
i18n (1.14.8) sha256=285778639134865c5e0f6269e0b818256017e8cde89993fdfcbfb64d088824a5
138138
json (2.19.8) sha256=6354310fd76ef69b87d5bd1f38b40d730613baf90b6803d2d0a48f618d32dfaa
139139
language_server-protocol (3.17.0.5) sha256=fd1e39a51a28bf3eec959379985a72e296e9f9acfce46f6a79d31ca8760803cc
140140
lint_roller (1.1.0) sha256=2c0c845b632a7d172cb849cc90c1bce937a28c5c8ccccb50dfd46a485003cc87
141141
logger (1.7.0) sha256=196edec7cc44b66cfb40f9755ce11b392f21f7967696af15d274dde7edff0203
142-
loofah (2.25.1) sha256=d436c73dbd0c1147b16c4a41db097942d217303e1f7728704b37e4df9f6d2e04
142+
loofah (2.25.2) sha256=2007f746959ac65552456e04b433e83deb22759ab38c838b4445c70e43425918
143143
minitest (5.27.0) sha256=2d3b17f8a36fe7801c1adcffdbc38233b938eb0b4966e97a6739055a45fa77d5
144144
nokogiri (1.19.4-aarch64-linux-gnu) sha256=1269fb644a6de405057a53dd5c762b1209b43ca7424f839454d3dbc677c31a8f
145145
nokogiri (1.19.4-aarch64-linux-musl) sha256=35c65b9ce72b3bb03207bdbe7067915019dc18c1b9b59139684bd6690fdd01af

lib/rails/html/scrubbers.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ def scrub_attribute(node, attr_node)
172172
Loofah::HTML5::Scrub.scrub_attribute_that_allows_local_ref(attr_node)
173173
end
174174

175-
if Loofah::HTML5::SafeList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && attr_name == "xlink:href" && attr_node.value =~ /^\s*[^#\s].*/m
175+
if Loofah::HTML5::SafeList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && Loofah::HTML5::SafeList::SVG_HREF_ATTRIBUTES.include?(attr_name) && attr_node.value =~ /^\s*[^#\s].*/m
176176
attr_node.remove
177177
end
178178

rails-html-sanitizer.gemspec

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ Gem::Specification.new do |spec|
2525
spec.files = Dir["lib/**/*", "README.md", "MIT-LICENSE", "CHANGELOG.md"]
2626
spec.require_paths = ["lib"]
2727

28-
spec.add_dependency "loofah", "~> 2.25"
28+
spec.add_dependency "loofah", "~> 2.25", ">= 2.25.2"
2929

3030
# A fix was shipped in nokogiri v1.15.7 and v1.16.8 without which there is a vulnerability in this gem.
3131
spec.add_dependency "nokogiri", [">=1.15.7",

test/sanitizer_test.rb

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,44 @@ def test_scrubbing_svg_attr_values_that_allow_ref
911911
assert_equal(expected, actual)
912912
end
913913

914+
def test_use_xlink_href_local_ref_is_preserved
915+
input = '<svg><use xlink:href="#p"></use></svg>'
916+
actual = safe_list_sanitize(input, tags: %w(svg use), attributes: %w(href xlink:href))
917+
918+
assert_equal(input, actual)
919+
end
920+
921+
def test_use_xlink_href_external_ref_is_scrubbed
922+
input = '<svg><use xlink:href="http://bad.com/x#p"></use></svg>'
923+
expected = "<svg><use></use></svg>"
924+
actual = safe_list_sanitize(input, tags: %w(svg use), attributes: %w(href xlink:href))
925+
926+
assert_equal(expected, actual)
927+
end
928+
929+
def test_use_href_local_ref_is_preserved
930+
input = '<svg><use href="#p"></use></svg>'
931+
actual = safe_list_sanitize(input, tags: %w(svg use), attributes: %w(href))
932+
933+
assert_equal(input, actual)
934+
end
935+
936+
def test_use_href_external_ref_is_scrubbed
937+
input = '<svg><use href="http://bad.com/x#p"></use></svg>'
938+
expected = "<svg><use></use></svg>"
939+
actual = safe_list_sanitize(input, tags: %w(svg use), attributes: %w(href))
940+
941+
assert_equal(expected, actual)
942+
end
943+
944+
def test_use_href_external_ref_with_leading_newline_is_scrubbed
945+
input = %(<svg><use href="\nhttp://bad.com/x#p"></use></svg>)
946+
expected = "<svg><use></use></svg>"
947+
actual = safe_list_sanitize(input, tags: %w(svg use), attributes: %w(href))
948+
949+
assert_equal(expected, actual)
950+
end
951+
914952
def test_style_with_css_payload
915953
input, tags = "<style>div > span { background: \"red\"; }</style>", ["style"]
916954
actual = safe_list_sanitize(input, tags: tags)

0 commit comments

Comments
 (0)