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

Stored Cross-site Scripting Bypassing TinyMCE Editor #531

Closed
ProDigySML opened this Issue Nov 20, 2017 · 4 comments

Comments

2 participants
@ProDigySML

ProDigySML commented Nov 20, 2017

Description of the problem

Anyone who can edit an experiment can gain arbitrary execution of JavaScript, stored within an experiment.
All you have to do is use a web interception proxy, like Burp Suite, and intercept the saving of an experiment. Edit the infos section of the experiment and add in <img src=x onerror=alert(1)>. An alert box will show up, which proves the arbitrary execution of JavaScript code.

Information

  • Elabftw version (visible in Sysadmin panel) : 1.7.8
  • Installation method (git, docker or zip archive) : git
  • Operating system + version : Ubuntu 16.04
  • Browser used (firefox/chrome) : firefox

For git/zip installation method:

  • PHP version: 7.0
  • MySQL version: 5.7.20
  • Webserver (apache/nginx) + version : Apache 2.4.29
@NicolasCARPi

This comment has been minimized.

Show comment
Hide comment
@NicolasCARPi

NicolasCARPi Nov 20, 2017

Contributor

Hello,

Thank you for your report.

Most XSS should be filtered already. Unfortunately, it is very difficult to filter out everything. So my plan is to have a Content-Security-Policy header that disables inline JS. I've been porting all javascript to seperate files and removing all the "onclick" and such that were in the code in the last months. I think there is not much left to add this header. (the header is already present, it's just lax for JS)

Also, the example you give doesn't work in my hands. The onerror part is stripped. I used tamper data firefox addon to edit the POST request before submitting it.

I tried looking at burpsuite but I'm not too keen on installing their binary blob on my system…

Another issue to read: #227

I'll work on removing all inline JS :)

Contributor

NicolasCARPi commented Nov 20, 2017

Hello,

Thank you for your report.

Most XSS should be filtered already. Unfortunately, it is very difficult to filter out everything. So my plan is to have a Content-Security-Policy header that disables inline JS. I've been porting all javascript to seperate files and removing all the "onclick" and such that were in the code in the last months. I think there is not much left to add this header. (the header is already present, it's just lax for JS)

Also, the example you give doesn't work in my hands. The onerror part is stripped. I used tamper data firefox addon to edit the POST request before submitting it.

I tried looking at burpsuite but I'm not too keen on installing their binary blob on my system…

Another issue to read: #227

I'll work on removing all inline JS :)

@ProDigySML

This comment has been minimized.

Show comment
Hide comment
@ProDigySML

ProDigySML Nov 21, 2017

Hi @NicolasCARPi,

There is no need to install burpsuite (but just as an FYI, it is an industry standard for web application testing). A CSP fix should work well. Maybe pairing it with the X-XSS-Protection header would be a good idea too.

Finally the easiest fix for a vulnerability like this in my opinion is always HTML encode, however I can understand that may be hard in some cases. Hope that helps!

ProDigySML commented Nov 21, 2017

Hi @NicolasCARPi,

There is no need to install burpsuite (but just as an FYI, it is an industry standard for web application testing). A CSP fix should work well. Maybe pairing it with the X-XSS-Protection header would be a good idea too.

Finally the easiest fix for a vulnerability like this in my opinion is always HTML encode, however I can understand that may be hard in some cases. Hope that helps!

@NicolasCARPi

This comment has been minimized.

Show comment
Hide comment
@NicolasCARPi

NicolasCARPi Nov 21, 2017

Contributor

Thanks for your input @ProDigySML. The X-XSS-Protection is already here.

I'll focus on removing all inline JS to implement the header ASAP :)

FYI, it is an industry standard for web application testing

Standard or not, if it's not open source it's not going to get installed ;)

Contributor

NicolasCARPi commented Nov 21, 2017

Thanks for your input @ProDigySML. The X-XSS-Protection is already here.

I'll focus on removing all inline JS to implement the header ASAP :)

FYI, it is an industry standard for web application testing

Standard or not, if it's not open source it's not going to get installed ;)

@NicolasCARPi

This comment has been minimized.

Show comment
Hide comment
@NicolasCARPi

NicolasCARPi Nov 30, 2017

Contributor

Now the whole app is working with a restrictive CSP (without unsafe-inline and without unsafe-eval).

Please do not hesitate to tell me if you find other vulns. The app should be audited in the next weeks (or months) by paid professional anyway, but the more eyes, the better ;)

Closing this as inline JS will not be executed by browsers anymore.

Contributor

NicolasCARPi commented Nov 30, 2017

Now the whole app is working with a restrictive CSP (without unsafe-inline and without unsafe-eval).

Please do not hesitate to tell me if you find other vulns. The app should be audited in the next weeks (or months) by paid professional anyway, but the more eyes, the better ;)

Closing this as inline JS will not be executed by browsers anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment