Escape.jsString issue

[shrek19937]

I hope this is just due to my ignorance, but why is the semi-column (;) not escaped? It seemed open the door for attacks like this,

coverity jsp/el escaping
<!-- Example of usage within a JSP --> 
<!-- http://localhost:8080/HelloJSP/jsp/testcout.jsp?foo=2;alert(1);  -->
<script type="text/javascript">
    // var n1 = <%=Escape.jsString("1.5; alert(1);") %>;
    var n3 = <%=Escape.jsString(request.getParameter("foo")) %>;
</script>

Maybe I'm missing something here. Please advise.

[neuroo]

Hey, indeed this function is not meant to be use in that case. When you have such code:

<script>
  var foo = ${my_variable};
</script>

you cannot rely on a JavaScript string escaper since ${my_variable} will not appear inside a JavaScript string. In fact, you add ${my_variable} directly to the JavaScript code. In this case, there is no escaper that could solve your problem and you will need to use something like a whitelist to make sure that ${my_variable} contains only digits or so.

If ${my_variable} is meant to contain a string (and is supposedly going into a JavaScript string), then you need to surround your value with quotes:

<script>
  var foo = '${cov:jsStringEscape(my_variable)}';
</script>

Btw, I used the EL notations for the escaper because it's just easier to type. The same thing applies to scriptlet code with: var n3 = '<%=Escape.jsString(request.getParameter("foo")) %>';

[shrek19937]

HI Romain, I appreciate your quick response. Could you elaborate on why are you not escaping the semi-columns. (;). Is it for performance considerations? I'm working on a research project for my company to prevent xss and I really liked your library.

Michael

On Fri, Feb 1, 2013 at 6:01 PM, Romain Gaucher notifications@github.comwrote:

Hey, indeed this function is not meant to be use in that case. When you have such code:

you cannot rely on a JavaScript string escaper since ${my_variable} will not appear inside a JavaScript string. In fact, you add ${my_variable}directly to the JavaScript code. In this case, there is no escaper that could solve your problem and you will need to use something like a whitelist to make sure that ${my_variable} contains only digits or so.

If ${my_variable} is meant to contain a string (and is supposedly going into a JavaScript string), then you need to surround your value with quotes:

— Reply to this email directly or view it on GitHubhttps://github.com/coverity/coverity-security-library/issues/4#issuecomment-13018791.

[neuroo]

It's not a performance consideration, it's just that inside a javascript string the semicolon is not relevant (of course I'm talking about string that do not flow into an eval-like construct). To break out of the string, you need to close the quote, play with the new lines, or switch to a script double escaped (which is barely exploitable btw). So yeah, semicolon is just a nop for javascript string contexts.

[jpasski]

Hi Michael,

To expand on Romain's response,the tl;dr is that the context you're in suffers more than just escaping semicolons.

Long answer: JavaScript is comprised of many different types of contexts, each of which have certain security obligations. Common contexts that receive tainted data are single-quoted and double-quoted strings. JavaScript is based on ECMAScript (Standard ECMA-262). When writing the escapers, we reviewed Chapter 7 and the lexical breakdown from one element to another for some small number of contexts. This is where usually security issues occur. The jsString escaper is mainly designed to ensure only certain characters described in Section 7.8.4 are allowed. If you haven't read this section it's well worth the read. Some of those characters if injected without escaping would make the string not a string and something else. That is, they would escape the string context and enter probably normal JavaScript context.

The code sample you posted wasn't in a string context, single or double quoted. It wasn't really in any context. It was just plan old JavaScript. The thing about this context is that even semicolons are optional in browsers. Try the following:

    <script>
        var foo = 'bar'
        alert(foo)
    </script>

Escaping a semicolon doesn't prevent one from continuing onto another statement. If the foo variable is used in some way later, then there are other ways to end the line. // commenting, <!--<script> commenting, or whatever. Regardless of the approach, you're still in the JavaScript main context (rvalue but still). By wrapping the value in quotes, it suddenly changes the context to be something that's a lot easier to secure. Instead, being in raw JavaScript, pretty much every non-alphanumeric character would have to be filtered out. No escaping. Just filtered out. Then you'd have to do whitelists so someone can't do this:

    <script>
        var foo = TAINTED_DATA
        foo()
    </script>

Imaging if TAINTED_DATA was eval. Game over. It gets to be a harder problem. The escapers written are dealing with more tractable issues. There's a small set of characters that have any security obligation within a JavaScript string context. The escaper manages those and additional ones since some contexts are nested into other parent contexts. But the general rule is the same.

Good luck!

[shrek19937]

Thank you so much. Jon, Romain. I'm sure to study that.