Discussion of #4131: addslashes, javascript string literals, and (new) escapejs

[Jeremy Dunck]

Hello devs,
The current trunk addslashes filter is documented as "Adds slashes.
Useful for passing strings to JavaScript, for example."

However, there are many strings that can be passed through that
filter and sill will break javascript string literals.

4131 now has a patch (from Andy Durdin) which would introduce a new
defaultfilter named escapejs. It does the complete job of escaping
anything that could break out of a string literal.

The ticket is now marked "Design decision needed" in order to decide
whether it is better to add escapejs, modify addslashes instead, or
WONTFIX.

I think this is a potential security issue, since it can be used to
do things like this:

var x = '{{ vars }}'; rendered to:
var x = '
}</script><script>alert('muhaha!');</script>

My vote is for escapejs, since addslashes may be used for other
purposes and changing it would be (narrowly) backwards-incompatible.

I think the purpose of escapejs is pretty straight-forward.

Decision, please. :)

[Andrew Durdin]

On 10/17/07, Jeremy Dunck <jdu...@gmail.com> wrote:
>
> However, there are many strings that can be passed through that
> filter and sill will break javascript string literals.

Specifically, as you point out, strings that contain "</script>" --
the main point here is to reduce the chances of XSS attacks when
embedding user-originated data into scripts.

> 4131 now has a patch (from Andy Durdin) which would introduce a new
> defaultfilter named escapejs. It does the complete job of escaping
> anything that could break out of a string literal.

Credit where it's due; The meat of the patch is Jeremy's, I just
tidied it up a tad.

Andrew

[Ned Batchelder]

I liked the original proposal (mine!) to extend addslashes rather than to nearly-duplicate its functionality in another filter, but I can see the logic of slicing these things finely.  I like that the docs for addslashes refers to escapejs.  For the purposes of education, the escapejs docs should explain why they convert </ into <\/: a small mention there could go a long way to ensuring that future developers understand the problems at hand.

--Ned.

-- 
Ned Batchelder, http://nedbatchelder.com