diff --git a/lib/phlex/html.rb b/lib/phlex/html.rb index 5fa1eb74..e7c671d9 100644 --- a/lib/phlex/html.rb +++ b/lib/phlex/html.rb @@ -4,9 +4,6 @@ class Phlex::HTML < Phlex::SGML autoload :StandardElements, "phlex/html/standard_elements" autoload :VoidElements, "phlex/html/void_elements" - # A list of HTML attributes that have the potential to execute unsafe JavaScript. - UNSAFE_ATTRIBUTES = Set.new(%w[onabort onafterprint onbeforeprint onbeforeunload onblur oncanplay oncanplaythrough onchange onclick oncontextmenu oncopy oncuechange oncut ondblclick ondrag ondragend ondragenter ondragleave ondragover ondragstart ondrop ondurationchange onemptied onended onerror onfocus onhashchange oninput oninvalid onkeydown onkeypress onkeyup onload onloadeddata onloadedmetadata onloadstart onmessage onmousedown onmousemove onmouseout onmouseover onmouseup onmousewheel onoffline ononline onpagehide onpageshow onpaste onpause onplay onplaying onpopstate onprogress onratechange onreset onresize onscroll onsearch onseeked onseeking onselect onstalled onstorage onsubmit onsuspend ontimeupdate ontoggle onunload onvolumechange onwaiting onwheel srcdoc]).freeze - extend Phlex::SGML::Elements include VoidElements, StandardElements diff --git a/lib/phlex/sgml.rb b/lib/phlex/sgml.rb index 64065edb..6616781d 100644 --- a/lib/phlex/sgml.rb +++ b/lib/phlex/sgml.rb @@ -2,6 +2,9 @@ # **Standard Generalized Markup Language** for behaviour common to {HTML} and {SVG}. class Phlex::SGML + UNSAFE_ATTRIBUTES = Set.new(%w[srcdoc sandbox http-equiv]).freeze + REF_ATTRIBUTES = Set.new(%w[href src action formaction lowsrc dynsrc background ping]).freeze + autoload :Elements, "phlex/sgml/elements" autoload :SafeObject, "phlex/sgml/safe_object" autoload :SafeValue, "phlex/sgml/safe_value" @@ -357,62 +360,75 @@ def __attributes__(attributes, buffer = +"") else raise Phlex::ArgumentError.new("Attribute keys should be Strings or Symbols.") end - lower_name = name.downcase - - unless Phlex::SGML::SafeObject === v - if lower_name == "href" && v.to_s.downcase.delete("^a-z:").start_with?("javascript:") - next - end - - # Detect unsafe attribute names. Attribute names are considered unsafe if they match an event attribute or include unsafe characters. - if Phlex::HTML::UNSAFE_ATTRIBUTES.include?(lower_name.delete("^a-z-")) - raise Phlex::ArgumentError.new("Unsafe attribute name detected: #{k}.") - end - end - - if name.match?(/[<>&"']/) - raise Phlex::ArgumentError.new("Unsafe attribute name detected: #{k}.") - end - - if lower_name.to_sym == :id && k != :id - raise Phlex::ArgumentError.new(":id attribute should only be passed as a lowercase symbol.") - end - - case v + value = case v when true - buffer << " " << name + true when String - buffer << " " << name << '="' << v.gsub('"', """) << '"' + v.gsub('"', """) when Symbol - buffer << " " << name << '="' << v.name.tr("_", "-").gsub('"', """) << '"' + v.name.tr("_", "-").gsub('"', """) when Integer, Float - buffer << " " << name << '="' << v.to_s << '"' + v.to_s when Hash case k when :style - buffer << " " << name << '="' << __styles__(v).gsub('"', """) << '"' + __styles__(v).gsub('"', """) else __nested_attributes__(v, "#{name}-", buffer) end when Array case k when :style - buffer << " " << name << '="' << __styles__(v).gsub('"', """) << '"' + __styles__(v).gsub('"', """) else - buffer << " " << name << '="' << __nested_tokens__(v) << '"' + __nested_tokens__(v) end when Set case k when :style - buffer << " " << name << '="' << __styles__(v).gsub('"', """) << '"' + __styles__(v).gsub('"', """) else - buffer << " " << name << '="' << __nested_tokens__(v.to_a) << '"' + __nested_tokens__(v.to_a) end when Phlex::SGML::SafeObject - buffer << " " << name << '="' << v.to_s.gsub('"', """) << '"' + v.to_s.gsub('"', """) else raise Phlex::ArgumentError.new("Invalid attribute value for #{k}: #{v.inspect}.") end + + lower_name = name.downcase + + unless Phlex::SGML::SafeObject === v + normalized_name = lower_name.delete("^a-z-") + + if value != true && REF_ATTRIBUTES.include?(normalized_name) && value.downcase.delete("^a-z:").start_with?("javascript:") + # We just ignore these because they were likely not specified by the developer. + next + end + + if normalized_name.bytesize > 2 && normalized_name.start_with?("on") && !normalized_name.include?("-") + raise Phlex::ArgumentError.new("Unsafe attribute name detected: #{k}.") + end + + if UNSAFE_ATTRIBUTES.include?(normalized_name) + raise Phlex::ArgumentError.new("Unsafe attribute name detected: #{k}.") + end + end + + if name.match?(/[<>&"']/) + raise Phlex::ArgumentError.new("Unsafe attribute name detected: #{k}.") + end + + if lower_name.to_sym == :id && k != :id + raise Phlex::ArgumentError.new(":id attribute should only be passed as a lowercase symbol.") + end + + case value + when true + buffer << " " << name + when String + buffer << " " << name << '="' << value << '"' + end end buffer diff --git a/test/phlex/view/naughty_business.rb b/test/phlex/view/naughty_business.rb index 10ace5dd..5d56a389 100644 --- a/test/phlex/view/naughty_business.rb +++ b/test/phlex/view/naughty_business.rb @@ -157,7 +157,7 @@ def view_template end end - Phlex::HTML::UNSAFE_ATTRIBUTES.each do |event_attribute| + Phlex::SGML::UNSAFE_ATTRIBUTES.each do |event_attribute| with "with naughty #{event_attribute} attribute" do naughty_attributes = { event_attribute => "alert(1);" }