Is the following javascript safe from arbitrary code execution?

Question

I'm contributing to a javascript framework which has the equivalent of the following code:

eval("'" + user_input.replace(/'/g, "'") + "'");

I know this is terrible -- no need to persuade me. What I want to know is, can I inject arbitrary code here?

At first glance the user_input.replace("'", "'") would seem to prevent me from breaking out of the string. However I can pass in newlines e.g. \nalert(123)\n, but then the result is always a syntax error, e.g.

'
alert(123)
'

Is there actually a vector for code injection here, other than just causing a syntax error?

Answer 1

While this is undoubtedly a worrisome pattern, it's safe if used exactly in the way described. The only character that can terminate a single-quoted string in Javascript is the single quote character. So long as that character does not appear in the string interpolated into the single quotes, it cannot possibly be interpreted as anything other than a string.

About the worst thing I can think of that you could do is end a string with a backslash, which would result in an unterminated string, e.g. if user_input were:

example\

then the evaluated code would be

'example\'

which would result in a syntax error, because the string contained in the eval is never terminated. However, if the real eval is actually more complex, this is exploitable. For example, if the code were:

var escaped_input = user_input.replace(/'/g, "&39;");
eval("'" + escaped_input + "' some more stuff '" + escaped_input + "'");

then it could be exploited with an input like:

; alert(1); // \

which would result in:

'; alert(1); // \' some more stuff '; alert(1); // \'
                                      ^^^^^^^^^

in which the underlined content would be evaluated, because the quote that was supposed to exit the string was escaped, turning the next single quote into a closing quote! To be safe, I'd recommend escaping or replacing backslashes if possible (unless you're explicitly trying to use eval() to deal with them, in which case you might just catch the exception).