Skip to content

Commit

Permalink
Escape filename to avoid XSS from malicious input
Browse files Browse the repository at this point in the history
Because:

* If user input is provided for the file name (as in rendering an SVG
  based on a URL parameter), the blanket marking of the SVG output as
  HTML-safe exposes an app to an XSS attack in the comment listing the
  file that was not found.

Solution:

* HTML-escape the filename rendering the comment that it was not found.
  • Loading branch information
pbyrne committed Mar 3, 2020
1 parent f64cf53 commit f5363b3
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/inline_svg/action_view/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def read_svg(filename)

def placeholder(filename)
css_class = InlineSvg.configuration.svg_not_found_css_class
not_found_message = "'#{filename}' #{extension_hint(filename)}"
not_found_message = "'#{ERB::Util.html_escape_once(filename)}' #{extension_hint(filename)}"

if css_class.nil?
return "<svg><!-- SVG file not found: #{not_found_message}--></svg>".html_safe
Expand Down
11 changes: 11 additions & 0 deletions spec/helpers/inline_svg_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,17 @@ def transform(doc)
expect(output).to be_html_safe
end

it "escapes malicious input" do
malicious = "--></svg><script>alert(1)</script><svg>.svg"
allow(InlineSvg::AssetFile).to receive(:named).
with(malicious).
and_raise(InlineSvg::AssetFile::FileNotFound.new)

output = helper.send(helper_method, malicious)
expect(output).to eq "<svg><!-- SVG file not found: '#{ERB::Util.html_escape_once(malicious)}' --></svg>"
expect(output).to be_html_safe
end

it "gives a helpful hint when no .svg extension is provided in the filename" do
allow(InlineSvg::AssetFile).to receive(:named).
with('missing-file-with-no-extension').
Expand Down

0 comments on commit f5363b3

Please sign in to comment.