Skip to content

Commit

Permalink
Merge branch 'h1-2519936-foreign-ns-confusion' into flavorjones-2024-…
Browse files Browse the repository at this point in the history
…security-fixes

* h1-2519936-foreign-ns-confusion:
  fix: Namespace confusion when disallowing 'svg' or 'math'
  • Loading branch information
flavorjones committed Dec 1, 2024
2 parents d7a94c1 + f02ffbb commit 3fe22a8
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 2 deletions.
6 changes: 5 additions & 1 deletion lib/rails/html/scrubbers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,11 @@ def keep_node?(node)
end

def scrub_node(node)
node.before(node.children) unless prune # strip
# If a node has a namespace, then it's a tag in either a `math` or `svg` foreign context,
# and we should always prune it to avoid namespace confusion and mutation XSS vectors.
unless prune || node.namespace
node.before(node.children)
end
node.remove
end

Expand Down
64 changes: 63 additions & 1 deletion test/sanitizer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ def test_combination_of_svg_and_style_with_script_payload
# libxml2
"<svg><style>&lt;script&gt;alert(1)&lt;/script&gt;</style></svg>",
# libgumbo
"<svg><style>alert(1)</style></svg>"
"<svg><style></style></svg>",
]

assert_includes(acceptable_results, actual)
Expand Down Expand Up @@ -1004,6 +1004,48 @@ def test_combination_of_math_and_style_with_escaped_img_payload
assert_includes(acceptable_results, actual)
end

def test_combination_of_style_and_disallowed_svg_with_script_payload
# https://hackerone.com/reports/2519936
input, tags = "<svg><style><style class='</style><script>alert(1)</script>'>", ["style"]
actual = safe_list_sanitize(input, tags: tags)
acceptable_results = [
# libxml2
"<style>&lt;style class='</style>alert(1)'&gt;",
# libgumbo
"",
]

assert_includes(acceptable_results, actual)
end

def test_combination_of_style_and_disallowed_math_with_script_payload
# https://hackerone.com/reports/2519936
input, tags = "<math><style><style class='</style><script>alert(1)</script>'>", ["style"]
actual = safe_list_sanitize(input, tags: tags)
acceptable_results = [
# libxml2
"<style>&lt;style class='</style>alert(1)'&gt;",
# libgumbo
"",
]

assert_includes(acceptable_results, actual)
end

def test_math_with_disallowed_mtext_and_img_payload
# https://hackerone.com/reports/2519941
input, tags = "<math><mtext><table><mglyph><style><img src=: onerror=alert(1)>", ["math", "style"]
actual = safe_list_sanitize(input, tags: tags)
acceptable_results = [
# libxml2
"<math><style>&lt;img src=: onerror=alert(1)&gt;</style></math>",
# libgumbo
"<math></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 @@ -1113,5 +1155,25 @@ def test_should_not_be_vulnerable_to_nokogiri_foreign_style_serialization_bug

assert_nil(xss)
end

def test_should_not_be_vulnerable_to_ns_confusion_2519936
# https://hackerone.com/reports/2519936
input = "<math><style><style class='</style><script>alert(1)</script>'>"
result = Rails::HTML5::SafeListSanitizer.new.sanitize(input, tags: ["style"])
browser = Nokogiri::HTML5::Document.parse(result)
xss = browser.at_xpath("//script")

assert_nil(xss)
end

def test_should_not_be_vulnerable_to_ns_confusion_2519941
# https://hackerone.com/reports/2519941
input = "<math><mtext><table><mglyph><style><img src=: onerror=alert(1)>"
result = Rails::HTML5::SafeListSanitizer.new.sanitize(input, tags: %w(math 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 3fe22a8

Please sign in to comment.