Ticket #60 (new defect)
Opened 4 months ago
some xss preventions
| Reported by: | mkristian | Owned by: | somebody |
|---|---|---|---|
| Priority: | major | Milestone: | |
| Component: | lib | Version: | |
| Keywords: | Cc: |
Description
I am quite fascinated about markaby, just became my favorite "markup" language in no time. but it is very easy to build in xss vulnerabilities. I located three places where things can go wrong. of course "good" programming habits might help preventing it - I tried it - but enforcing these habits is much better.
* arguments to helper are not escaped, i.e.
link_to(params[:name], post_url)
can be used for html injection
* text, concat and << do not escape the given string, which is a feature but then you need to do the escaping manually
* &block which evaluates to a string do not get any escaping, i.e.
b { params[:name] }
I prepared a patch (or until now some code) which does the following
* escape the arguments for helpers, when markaby is configured to do so
* add an method text! (following the api of the underlying xml_builder) which escapes the string argument
* if configured, check text, concat and << whether they might allow html injection. same check for &blocks.
the idea of the check is easy: if you use the un-escaped variant of the method text than you should have an html fragment as argument. otherwise this fragment is a string than embedded user input can inject html, so raise an error, saying use the text! method. this leads to following situations:
* embed the html produced by a helper use text, concat or <<
* embed a plain string (with or without user input) use text!
* instead of
b { "something" }
use
b( "something")
in some cases the check does not work because it depends whether there is plain string or a html fragment. the check can be bypassed by a bypass_xss_check method, optional with a boolean condition to follow both cases.
of course this is not bullet proof, but I found it helpful for my little projects.