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

PrismJS is outdated and prone to DOM XSS #1123

Closed
BradHolmes opened this issue Jul 27, 2022 · 6 comments · Fixed by #1149
Closed

PrismJS is outdated and prone to DOM XSS #1123

BradHolmes opened this issue Jul 27, 2022 · 6 comments · Fixed by #1149
Assignees
Labels
dependencies Pull requests that update a dependency file e0-minutes Effort < 60 min e1-hours p1-high

Comments

@BradHolmes
Copy link

BradHolmes commented Jul 27, 2022

A checkmarx scan of our docsy-themed documentation turned up a reported Client DOM XSS vulnerability in static/js/prims.js. It reports:

The application's o embeds untrusted data in the generated output with element, at line 120 of .../static/js/prism.js. This untrusted data is embedded straight into the output without proper sanitization or encoding, enabling an attacker to inject malicious code into the output.

The problematic 'o' method:

function o(e) {
    l.highlightedCode =  e, M.hooks.run("before-insert", l), l.element.innerHTML = l.highlightedCode, M.hooks.run("after-highlight", l), M.hooks.run("complete", l), t && t.call(l.element)

I see that static/js/prism.js appears to be a hard-coded copy of approximately v1.21.0 of primsjs. But it isn't clear where exactly it came from. Should it be pulled in dynamically as an npm dependency? Or alternatively, a small change is made to static/js/prims.js to sanitize e before inserting it into the DOM?

@LisaFC
Copy link
Collaborator

LisaFC commented Jul 27, 2022

@chalin what do you think?

@chalin chalin added dependencies Pull requests that update a dependency file e0-minutes Effort < 60 min e1-hours p1-high labels Jul 27, 2022
@chalin
Copy link
Collaborator

chalin commented Jul 27, 2022

Thanks @BradHolmes for reporting this. Yes we should fix this. I'd vote for switching over to using a CDN. Any objections @LisaFC?

@LisaFC
Copy link
Collaborator

LisaFC commented Jul 27, 2022

No objections here!

@emckean
Copy link
Collaborator

emckean commented Jul 27, 2022

+1 for CDN

@BradHolmes
Copy link
Author

Is there anything I can do to help make this switch to CDN?

@chalin
Copy link
Collaborator

chalin commented Aug 9, 2022

Looking into this a bit more, and reading about how Docsy provides Prism support, I'd like to suggest that we simply update the Prism JS and CSS files to the current version (1.28.0 atm).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file e0-minutes Effort < 60 min e1-hours p1-high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants