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.

Note: See TracTickets for help on using tickets.