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

Security: XSS and prototype pollution from legacy jQuery #362

Open
hedsnz opened this issue Nov 22, 2022 · 4 comments
Open

Security: XSS and prototype pollution from legacy jQuery #362

hedsnz opened this issue Nov 22, 2022 · 4 comments

Comments

@hedsnz
Copy link

hedsnz commented Nov 22, 2022

The following XSS and prototype pollution vulnerabilities are present in the legacy version of jQuery included in pander (v1.7.2):

It appears that jquery.min.js is required for slimbox2.js which is called in custom.js. All three are included in inst/includes/html/header.html.

Is it possible to update jQuery to 3.6.x? I'm happy to submit a PR for this, let me know.

@daroczig
Copy link
Member

Thanks a lot, a PR would be highly appreciated 🙇

@hedsnz
Copy link
Author

hedsnz commented Jan 15, 2023

I've looked into slimbox2.js a bit further, and it hasn't been updated since 2015. It probably isn't compatible with jQuery 3, either. And if I inspect the browser console after this MRE,

library(pander)
myReport <- Pandoc$new()
myReport$format <- "html"
myReport$add(plot(1:10))
myReport$export()

I'm getting several JS uncaught syntax errors, including from slimbox2.js and jquery-1.7.2.min.js.

So I guess there are a couple of questions. Is slimbox working/doing anything in the current version? Is there a MRE showing it working? And if not, can it just be removed entirely?

If that's the case then I'll submit a PR with slimbox removed and jQuery updated to 3.x. It would also be good to understand what other components of pander rely on jQuery, though, for testing/update purposes.

Thanks

@daroczig
Copy link
Member

Thank you very much for raising this, @hedsnz!

I think this might indeed be a bit larger task after all, as the CSS template and JS functions have not been updated for years ... and the CDN that used to support the pander package at cdn.rapporter.net has also been gone in the past years.

Although I can revive that latter if needed, but a proper review would make much more sense now, as the whole HTML report structure was created 10 years (!) ago.

Anyway, putting aside the scope creep problem and focusing on your original question: I feel OK about dropping slimbox2.js from the project, and update jQuery. Once slimbox is gone, the only jQuery-based stuff remaining is the menu builder as per https://github.com/Rapporter/pander/blob/master/inst/includes/javascripts/custom.js -- probably not affected by the update.

hedsnz added a commit to hedsnz/pander that referenced this issue Mar 1, 2023
@hedsnz
Copy link
Author

hedsnz commented Mar 1, 2023

#364

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