XSS (mXSS) via malformed HTML attribute name in Action View tag helpers

Proof-of-concept (PoC) exploit path before the fix (illustrative, not executable against your environment): Assume an attacker provides a set of HTML attributes to a Rails tag helper, including a blank key intended to inject an attacker-controlled payload via an attribute-like string: Ruby (illustrative): # Before the fix, a blank-key attribute could be emitted by tag helpers html_before = ActionView::Helpers::TagHelper.tag("img", { "src" => "/safe.png", "" => "onerror=alert(1)" }) # Potential (malformed) output guess (depends on implementation details): # <img src="/safe.png" ""="onerror=alert(1)" /> In a browser, this could yield a malformed attribute structure that an attacker might craft around, potentially tainting HTML parsing or triggering unintended behaviors in downstream sanitizers or renderers (a form of mXSS). The exact exploit surface depends on how the surrounding HTML/JS sanitization handles empty attribute names, but the risk is that an empty attribute name could bypass checks or alter parsing in surprising ways. After the fix (what you should test in your environment): Ruby (post-fix expectation): html_after = ActionView::Helpers::TagHelper.tag("img", { "src" => "/safe.png", "" => "onerror=alert(1)" }) # Expected sanitized output (blank key skipped): # <img src="/safe.png" /> Demonstration steps you can run locally (conceptual): 1) Build an HTML fragment with a tag helper including a blank key along with a safe attribute, e.g. a data or aria hash as in the tests. 2) Render it via the Rails tag helper. 3) Inspect the resulting HTML to verify the blank-key attribute is not present. 4) Use a DOM parser (like Nokogiri) to assert that no attribute with an empty name exists. 5) Optionally inject a payload in a non-blank attribute (e.g., data-foo) to confirm normal attribute rendering remains unaffected. Ruby snippet you can adapt for testing (conceptual): require 'action_view' include ActionView::Helpers::TagHelper html_before = tag("img", { "src" => "/safe.png", "" => "onerror=alert(1)" }) # Inspect html_before to see if an empty-name attribute appears puts html_before # After fix, the same call should omit the blank key html_after = tag("img", { "src" => "/safe.png", "" => "onerror=alert(1)" }) puts html_after # Parsing with Nokogiri to confirm no empty-name attribute require 'nokogiri' frag = Nokogiri::HTML5::DocumentFragment.parse(html_after) attrs = frag.at_css('img')&.attributes&.keys || [] puts attrs.inspect

diff --git a/actionview/lib/action_view/helpers/tag_helper.rb b/actionview/lib/action_view/helpers/tag_helper.rb index 5d57eee6aedd6..bbbe4dfb03230 100644 --- a/actionview/lib/action_view/helpers/tag_helper.rb +++ b/actionview/lib/action_view/helpers/tag_helper.rb @@ -237,16 +237,19 @@ def tag_options(options, escape = true) # :nodoc: output = +"" sep = " " options.each_pair do |key, value| + next if key.blank? + type = TAG_TYPES[key] if type == :data && value.is_a?(Hash) value.each_pair do |k, v| - next if v.nil? + next if k.blank? || v.nil? + output << sep output << prefix_tag_option(key, k, v, escape) end elsif type == :aria && value.is_a?(Hash) value.each_pair do |k, v| - next if v.nil? + next if k.blank? || v.nil? case v when Array, Hash diff --git a/actionview/test/template/tag_helper_test.rb b/actionview/test/template/tag_helper_test.rb index 2c13fe101fe3e..1b735620d33e1 100644 --- a/actionview/test/template/tag_helper_test.rb +++ b/actionview/test/template/tag_helper_test.rb @@ -107,6 +107,27 @@ def test_tag_options_accepts_blank_option assert_equal "<p included=\"\" />", tag("p", included: "") end + def test_tag_options_rejects_blank_key + assert_equal "<p />", tag("p", "" => "value") + assert_equal "<p />", tag("p", nil => "value") + assert_equal '<p class="a" />', tag("p", "" => "value", "class" => "a") + assert_equal '<p class="a" />', tag("p", nil => "value", "class" => "a") + end + + def test_tag_options_rejects_blank_data_key + assert_equal "<p />", tag("p", data: { "" => "value" }) + assert_equal "<p />", tag("p", data: { nil => "value" }) + assert_equal '<p data-x="y" />', tag("p", data: { "" => "value", "x" => "y" }) + assert_equal '<p data-x="y" />', tag("p", data: { nil => "value", "x" => "y" }) + end + + def test_tag_options_rejects_blank_aria_key + assert_equal "<p />", tag("p", aria: { "" => "value" }) + assert_equal "<p />", tag("p", aria: { nil => "value" }) + assert_equal '<p aria-x="y" />', tag("p", aria: { "" => "value", "x" => "y" }) + assert_equal '<p aria-x="y" />', tag("p", aria: { nil => "value", "x" => "y" }) + end + def test_tag_builder_options_accepts_blank_option assert_equal "<p included=\"\"></p>", tag.p(included: "") end @@ -205,6 +226,14 @@ def test_tag_with_dangerous_unknown_attribute_name tag("the-name", { COMMON_DANGEROUS_CHARS => "the value" }, false, false) end + def test_tag_with_blank_attribute_name_generates_valid_markup + # https://hackerone.com/reports/3078929 + html = tag("img", "src" => "/nonexistent.png", "" => "/onerror=alert(1)") + fragment = Nokogiri::HTML5::DocumentFragment.parse(html) + attrs = fragment.at_css("img").attribute_nodes.map(&:name) + assert_equal [ "src" ], attrs + end + def test_tag_builder_with_dangerous_unknown_attribute_name escaped_dangerous_chars = "_" * COMMON_DANGEROUS_CHARS.size assert_equal "<the-name #{escaped_dangerous_chars}=\"the value\"></the-name>",