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

Information disclosure vulnerability in version 0.8.5 and earlier #2152

Closed
keksa opened this issue May 11, 2020 · 11 comments · Fixed by #2215
Closed

Information disclosure vulnerability in version 0.8.5 and earlier #2152

keksa opened this issue May 11, 2020 · 11 comments · Fixed by #2215
Labels
Milestone

Comments

@keksa
Copy link

keksa commented May 11, 2020

There is a chroot option, which should prevent dompdf from accessing files outside of configured root. However this option affects only the check for initial load of HTML in the "loadHtmlFile" method.

I can use the file protocol to load images (and stylesheets) outside of configured root without problems. For example <img src="file:///usr/lib/node_modules/npm/node_modules/qrcode-terminal/example/basic.png"> will load and display the image in generated PDF even though chroot is set to /usr/src/myapp/vendor/dompdf/dompdf.

This is a problem for us, because we allow (authenticated) users to configure their own PDF templates (for example for invoices). Meaning if someone inputs an image with a file protocol URL, they can access images that don't belong to them.

@bsweeney
Copy link
Member

We'll review the information you provided and get back to you.

@bsweeney bsweeney added this to the 0.8.6 milestone May 11, 2020
@keksa
Copy link
Author

keksa commented May 18, 2020

Hey @bsweeney, is there any update with this? 🙂

@bsweeney
Copy link
Member

Not yet but we are looking at it for the next release. Life is ... hectic ... right now.

@bsweeney bsweeney modified the milestones: 0.8.6, 0.8.7 Aug 21, 2020
@bsweeney bsweeney changed the title security issue Information disclosure vulnerability in version 0.8.5 and earlier Aug 23, 2020
@bsweeney
Copy link
Member

bsweeney commented Aug 24, 2020

The parsing logic has been updated with extra security. The adapter class and the bundled CPDF library are excluded because the parsing logic intermediates calls to those classes. A user should only have access to the CPDF class/adapter if they can execute PHP directly, in which case the extra security we've added is pointless.

@bsweeney bsweeney linked a pull request Aug 24, 2020 that will close this issue
@bsweeney
Copy link
Member

@keksa thanks for bringing this issue to our attention. I have a few more things to look at related to resource handling, but the changes currently posted in PR #2215 should address your concerns. If you have time please take a look.

I'll update this ticket with details on the issue once the 0.8.6 release is ready.

@keksa
Copy link
Author

keksa commented Aug 24, 2020

@bsweeney I can confirm this fixes the issue with our usecase, thank you.

@peter-gribanov
Copy link

This is a serious problem, but why did you break backward compatibility? You've broken a lot of your clients' code. Why was it impossible to add deprecation warnings and break backward compatibility at least in the minor version?
Please do not break backward compatibility in the patch anymore.

@bsweeney
Copy link
Member

@peter-gribanov this has been brought up elsewhere. This was an oversight on my part and I apologize. We will try to do better in the future.

@stollr
Copy link
Contributor

stollr commented Nov 10, 2020

@bsweeney I have extended the documentation so that it is easier for users to spot their issues with this restriction: https://github.com/dompdf/dompdf/wiki/Usage#security-restrictions-for-local-files

I hope this will help.

@bsweeney
Copy link
Member

Thanks for that @Naitsirch! I'm still hoping to build out a bit more documentation around the issue and any help is appreciated.

@stollr
Copy link
Contributor

stollr commented Nov 10, 2020

Three further things may help:

  1. Add the chroot option setting to the first usage example in the wiki. This will teach the users to specify that setting as soon as they start using dompdf.
  2. Add the method to the method summary.
  3. Search for code snippets in stackoverflow.com and add the chroot option setting.

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

Successfully merging a pull request may close this issue.

4 participants