Skip to content

Commit f02ffbb

Browse files
committed
fix: Namespace confusion when disallowing 'svg' or 'math'
- https://hackerone.com/reports/2519936 - https://hackerone.com/reports/2519941 Also, test that this solution addresses namespace confusion and DOM mutation with 'math' + 'mtext'.
1 parent 5104ca9 commit f02ffbb

File tree

2 files changed

+68
-2
lines changed

2 files changed

+68
-2
lines changed

lib/rails/html/scrubbers.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,11 @@ def keep_node?(node)
101101
end
102102

103103
def scrub_node(node)
104-
node.before(node.children) unless prune # strip
104+
# If a node has a namespace, then it's a tag in either a `math` or `svg` foreign context,
105+
# and we should always prune it to avoid namespace confusion and mutation XSS vectors.
106+
unless prune || node.namespace
107+
node.before(node.children)
108+
end
105109
node.remove
106110
end
107111

test/sanitizer_test.rb

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,7 @@ def test_combination_of_svg_and_style_with_script_payload
918918
# libxml2
919919
"<svg><style>&lt;script&gt;alert(1)&lt;/script&gt;</style></svg>",
920920
# libgumbo
921-
"<svg><style>alert(1)</style></svg>"
921+
"<svg><style></style></svg>",
922922
]
923923

924924
assert_includes(acceptable_results, actual)
@@ -976,6 +976,48 @@ def test_combination_of_svg_and_style_with_img_payload_2
976976
assert_includes(acceptable_results, actual)
977977
end
978978

979+
def test_combination_of_style_and_disallowed_svg_with_script_payload
980+
# https://hackerone.com/reports/2519936
981+
input, tags = "<svg><style><style class='</style><script>alert(1)</script>'>", ["style"]
982+
actual = safe_list_sanitize(input, tags: tags)
983+
acceptable_results = [
984+
# libxml2
985+
"<style>&lt;style class='</style>alert(1)'&gt;",
986+
# libgumbo
987+
"",
988+
]
989+
990+
assert_includes(acceptable_results, actual)
991+
end
992+
993+
def test_combination_of_style_and_disallowed_math_with_script_payload
994+
# https://hackerone.com/reports/2519936
995+
input, tags = "<math><style><style class='</style><script>alert(1)</script>'>", ["style"]
996+
actual = safe_list_sanitize(input, tags: tags)
997+
acceptable_results = [
998+
# libxml2
999+
"<style>&lt;style class='</style>alert(1)'&gt;",
1000+
# libgumbo
1001+
"",
1002+
]
1003+
1004+
assert_includes(acceptable_results, actual)
1005+
end
1006+
1007+
def test_math_with_disallowed_mtext_and_img_payload
1008+
# https://hackerone.com/reports/2519941
1009+
input, tags = "<math><mtext><table><mglyph><style><img src=: onerror=alert(1)>", ["math", "style"]
1010+
actual = safe_list_sanitize(input, tags: tags)
1011+
acceptable_results = [
1012+
# libxml2
1013+
"<math><style>&lt;img src=: onerror=alert(1)&gt;</style></math>",
1014+
# libgumbo
1015+
"<math></math>",
1016+
]
1017+
1018+
assert_includes(acceptable_results, actual)
1019+
end
1020+
9791021
def test_should_sanitize_illegal_style_properties
9801022
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;)
9811023
expected = %(display:block;width:100%;height:100%;background-color:black;background-x:center;background-y:center;)
@@ -1075,5 +1117,25 @@ class HTML4SafeListSanitizerTest < Minitest::Test
10751117
class HTML5SafeListSanitizerTest < Minitest::Test
10761118
@module_under_test = Rails::HTML5
10771119
include SafeListSanitizerTest
1120+
1121+
def test_should_not_be_vulnerable_to_ns_confusion_2519936
1122+
# https://hackerone.com/reports/2519936
1123+
input = "<math><style><style class='</style><script>alert(1)</script>'>"
1124+
result = Rails::HTML5::SafeListSanitizer.new.sanitize(input, tags: ["style"])
1125+
browser = Nokogiri::HTML5::Document.parse(result)
1126+
xss = browser.at_xpath("//script")
1127+
1128+
assert_nil(xss)
1129+
end
1130+
1131+
def test_should_not_be_vulnerable_to_ns_confusion_2519941
1132+
# https://hackerone.com/reports/2519941
1133+
input = "<math><mtext><table><mglyph><style><img src=: onerror=alert(1)>"
1134+
result = Rails::HTML5::SafeListSanitizer.new.sanitize(input, tags: %w(math style))
1135+
browser = Nokogiri::HTML5::Document.parse(result)
1136+
xss = browser.at_xpath("//img/@onerror")
1137+
1138+
assert_nil(xss)
1139+
end
10781140
end if loofah_html5_support?
10791141
end

0 commit comments

Comments
 (0)