GrapesJS / grapesjs

Free and Open source Web Builder Framework. Next generation tool for building templates without coding
https://grapesjs.com
BSD 3-Clause "New" or "Revised" License
22.36k stars 4.05k forks source link

BUG: XSS vulnerability via component attributes #4076

Closed diemkay closed 2 years ago

diemkay commented 2 years ago

GrapesJS version

What browser are you using?

Chrome 97.0.4692.71

Reproducible demo link

https://jsfiddle.net/ovrz5ug2/4/

Describe the bug

Hi - we ran across this XSS vulnerability while using GrapesJS in a multiplayer type scenario, with several privileged users able to make changes to templates and components via the editor (i.e., non-devs). Effectively, this renders them vulnerable to an attack from within the organization, where one user adds malicious code, and a different one can come across it and run it.

How to reproduce the bug?

  1. Add malicious code into a component's id attribute, either directly in the HTML or via the trait manager (and save) - in this case, id="<details/open/ontoggle=alert(document.location)>
  2. Click on the component in the live preview and observe the alert OR via the Style Manager, when using the component state dropdown and changing it to a different state, e.g., hover.

What is the expected behavior? No XSS via the component's attributes (including, but not limited to id).

What is the current behavior? GrapesJS runs the malicious code.

If is necessary to execute some code in order to reproduce the bug, paste it here below:

// see above, adding this code to the component's attribute, e.g., id
<details/open/ontoggle=alert(document.location)>

If you have any tips or guidance, I could lend a hand with a PR.

Code of Conduct

artf commented 2 years ago

Thanks @diemkay please refer to this issue if you have any suggestions: https://github.com/artf/grapesjs/issues/3082

diemkay commented 2 years ago

@artf Thanks, but I've already seen that ticket and it doesn't cover the issue I'm describing here.

The injection is not in Live Preview, it's in Style Manager, where it tries to display the id of the component, by setting .innerHtml.

artf commented 2 years ago

Yeah sorry, closed too soon 😁. I'll try to fix for the next release.

bberenberg commented 2 years ago

@artf can you share a timeline for when the next release will be?

artf commented 2 years ago

I hope to release it this week.