Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xss injection #20

Open
espretto opened this issue Feb 1, 2021 · 2 comments
Open

xss injection #20

espretto opened this issue Feb 1, 2021 · 2 comments

Comments

@espretto
Copy link

espretto commented Feb 1, 2021

Hi,
The getFallbackHTML method does not escape the json for html here. It should escape the json before sprintf-ing it into html tags just like its js counterpart here.

I'd be happy to contribute if PRs are welcome.

@roippi
Copy link
Collaborator

roippi commented Feb 5, 2021

Hi, thanks for the feedback. After thinking about this for a sec, you are likely correct, since the json-encoded data is put into an HTML comment block, which could be ended by malicious stuff in the data.

However I don't agree with the js counterpart's approach; brute force replacing all & and > in the input with the corresponding html entities isn't a palatable solution. I could envision scenarios where I have those in my props and I don't want them to turn into htmlentities.

Happy to think through solutions here if you want to provide a test case.

@espretto
Copy link
Author

espretto commented Feb 8, 2021

The client-side decoder expects server-rendered JSON to be encoded. The client first reverses the conversion and only then parses to JSON. The server should mirror the decoder. Example:

str_replace(['&', '>'], ['&', '>'], json_encode(data));

Unfortunately, there's no flag available to make htmlspecialchars do this. The other special chars are not needed because only > can terminate an html comment (spec). Let's say the above expression is returned by a function hypernova_encode, here is how it compares to json_encode:

$data = ['xss' => 'comment--><script>window.close()</script>'];

sprintf('<script type="application/json"><!--%s--></script>', json_encode($data));
// <script type="application/json"><!--{"xss":"comment--><script>window.close()</script>"}--></script>

sprintf('<script type="application/json"><!--%s--></script>', hypernova_encode($data));
// <script type="application/json"><!--{"xss":"comment--&gt;<script-&gt;window.close()</script-&gt;"}--></script>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants