Fix #11: Support double quotes in strings

[arendjr]

This PR makes it possible to use double quotes inside strings, provided they're escaped with a preceding backslash.

Please note this PR could be considered breaking, as literal backslashes will now be interpreted as escape characters.

[cshaa]

Looks good to me! Just give me some more time to read the code properly before merging.

[cshaa]

Your regex also matches "a\" as a legit string. The not-char pattern should be something like [^"\\] instead of [^"]. This can be used as an exploit, because the next string would be actually executed as JavaScript:

"a\" == "; window.p0wned = true; //"

Also, can you guarantee that removing the JSON.stringify won't result in any malicious code? We know that everything that comes out of stringify is safe. But how can we be sure every match of your regex is safe? (After fixing the previous bug, of course, now it obviously isn't safe.)

[cshaa]

I've found a potential security bug in your code, please be sure to fix it before I merge the PR.
Furthermore, you seem to have pushed from a branch where more people have write access, and @albehrens, probably unknowingly, also pushed there. While his commit is interesting, I'd like to discuss it in a separate PR.

[albehrens]

@m93a arend and I work in a team. That is why we both have access to the repo. I reverted my commit and open a new PR: #16

[arendjr]

Thanks for the detailed explanation! I added tests for these specific cases, but actually the regex was already not vulnerable to this. It seems to me this was because the first alternative in the parentheses (\\\\") would swallow the characters (\") and thereby the double quotes would not be allowed to close the string. I have added these test cases to the PR.

Even so, it is true the regex would pretty much allow all JS escape codes to sneak in. I did not see much issue with this, though I admit I can also not guarantee it to be safe. So I have tightened the regex, so that only \\ and \" are allowed as valid escape sequences. Sequences such as \n are still invalid now. I have added test cases for this too.

Adding JSON.stringify() is unfortunately not an option. This would insert additional backslashes that would break the resulting output. Still, by only allowing explicitly allowing the \\ and \" escape codes, I hope to have made it sufficiently secure.

[arendjr]

Thank you!

[cshaa]

@arendjr Don't you mind being added to the list of contributors in README?

[arendjr]

Not at all 👍