TwisterMc / pdfjs-viewer-shortcode

A Wordpress plugin for embedding PDFs using Mozilla's Excellent PDF.js
https://wordpress.org/plugins/pdfjs-viewer-shortcode/
Apache License 2.0
7 stars 4 forks source link

Security/Compatibility Concern: Plugin is calling PHP file(s) directly #14

Open KZeni opened 3 years ago

KZeni commented 3 years ago

– The Problem – It appears pdfjs/web/viewer.php is being called directly & that’s problematic when WordPress hosting/sites are properly hardened to prevent that potentially harmful attack vector.

It’s a WordPress theme & plugin guideline to not call PHP files directly as this can be a serious security issue. Instead, the files should be included and then have their functions/hooks/etc. called via the WordPress system (which then has better control & view into what’s being done for security purposes & code interoperability as well as certifying that the code being ran wasn’t just some random file that was uploaded & is really part of the plugin/system.)

As such, there are actually WordPress hosting providers, plugins (Sucuri being one of many), and configurations that specifically disable the ability of a PHP file located in a theme or plugin from being called directly. This importantly makes it so a malicious PHP file that might somehow be uploaded to the site can’t then just be executed by visiting that file (per it then being blocked). This then, unfortunately, blocks parts of plugins/themes that don’t follow the guideline & just have PHP file(s) being called directly (when that’s totally avoidable as mentioned above.)

– Potential Fix – In the case of viewer.php, I see no reason it can’t have its code included that's called via more WordPress-integrated means (ex. as a virtual page URL that the plugin makes available which effectively serves as an alias for viewer.php to show the plugin officially expects that to be executed & isn't just a PHP file that could've potentially been injected into the site to then be ran as potentially malicious standalone code [hence Sucuri & others offering to block standalone PHP file execution within the wp-content folder, etc.])

There might be cases outside of viewer.php, but that one is for sure actively problematic, at the moment.

Again, this is an important security precaution where this direct PHP file being called should be redone. Also, this means the plugin is actively breaking on assorted hosting/setups where they have things hardened against this potential attack vector as a whole.

https://wordpress.org/support/article/hardening-wordpress/#code-execution-plugins specifically calls out this guideline and details officially recommended way (have it display a page like any other and adapt it and/or only output things as needed for what’s being shown [assuming it isn’t otherwise an admin-ajax.php related function instead of a page-style output]) to avoid this problem.

Sites with Sucuri have a workaround for now where you can whitelist PHP file execution for viewer.php within the wp-content folder, but that is an added step to make it so the plugin doesn't break that otherwise could be avoided through conventions WordPress documentation has outlined (per the link above.)

I've also posted this on the WP.org support forum just in case others experience this & don't know to also check GitHub: https://wordpress.org/support/topic/security-compatibility-concern-plugin-is-calling-php-files-directly/

Thanks for the great plugin. I just want to make sure this plugin is kept secure & doesn't break when sites/hosting harden their security in this particular way.

TwisterMc commented 2 years ago

I really do need to figure out how to create a virtual page. I just went through an audit from WordPress and had to fix a lot of things. Hopefully it's better, but it is something I need to figure out.