search

Sucuri / sucuri-wordpress-plugin

WordPress Plugin - Auditing, Malware Scanner and Security Hardening
GNU General Public License v2.0
49 stars 30 forks source link

Issue with directory separators / file paths #118

Open pixelbrad opened 3 years ago

pixelbrad commented 3 years ago

Hello,

I'm working on a project locally on a Windows machine and had to test Sucuri. I discovered Sucuri is generating a PHP error before WordPress has fully initialized, even before debug logging is set.

In the function htaccess (sucuri-scanner/src/hardening.lib.php:191), $folder is passed in, but it has all forward slashes (for example: F:/sites/my-bedrock-project/web/app/sucuri). On line 193, we're trying to replace ABSPATH in the $folder string with nothing. The problem is, ABSPATH uses a mix of (mostly) backslashes and forward slashes: F:\sites\my-bedrock-project\web\wp/. Ultimately, the htaccess function returns a string like this:

F:\sites\my-bedrock-project\web\wp//F:/sites/my-bedrock-project/web/app/sucuri/.htaccess

I discovered this by digging in to why my WP admin dashboard had the class php-error but no errors were displayed or logged to the debug log. In wordpress/wp-admin/admin-header.php:197, error_get_last is called, and by viewing its output, it tells me Sucuri is trying to open a file it can't open:

fopen(F:\sites\my-bedrock-project\web\wp//F:/sites/my-bedrock-project/web/app/sucuri/.htaccess): failed to open stream: No such file or directory.

sucuri-scanner/src/hardening.lib.php is trying to open that file on line 98.

Extra Information

gvre commented 3 years ago

@pixelbrad Thanks a lot for the detailed report. Could you try #119 to see if it solves the issue?

pixelbrad commented 3 years ago

@gvre Thanks for the quick response! I won't be able to test this until mid/late August, but I will report back once I do.

pixelbrad commented 3 years ago

I've just had a moment to test this. I'm... not sure I'm getting the results I expected. The paths look better overall, but I'm still getting seeing an error.

(Note that I modified the code from the PR slightly just to make it easier to see what the htaccess function is returning)

Using WP_CONTENT_DIR

Using WP_CONTENT_DIR This is what I see when SUCURI_DATA_STORAGE is set to WP_CONTENT_DIR . '/sucuri'. WP_CONTENT_DIR comes back as F:\sites\my-bedrock-project/web/app.

Error message: fopen(F:/sites/my-bedrock-project/web/wp/F:/sites/my-bedrock-project/web/app/sucuri/.htaccess): failed to open stream: No such file or directory

Using CONTENT_DIR

Using CONTENT_DIR This is what I see when SUCURI_DATA_STORAGE is set to CONTENT_DIR . '/sucuri'. CONTENT_DIR comes back as /app.

Error message: fopen(F:/sites/my-bedrock-project/web/wp//app/sucuri/.htaccess): failed to open stream: No such file or directory

Not defining SUCURI_DATA_STORAGE at all

If I omit defining SUCURI_DATA_STORAGE, it defaults to using F:/sites/my-bedrock-project/web/app/uploads/sucuri, which isn't ideal for me in this case, hence why I want to define a custom location.

Error message: fopen(F:/sites/my-bedrock-project/web/wp/F:/sites/my-bedrock-project/web/app/uploads/sucuri/.htaccess): failed to open stream: No such file or directory

In my case, I will be launching this Bedrock project in multiple environments, including staging and production, and will be sharing the entire uploads folder between the multiple environments. So I don't want Sucuri logs/data spilling over from one environment to another. Ideally, I would like to use F:/sites/my-bedrock-project/web/app/sucuri as the storage directory - that's what I'm attempting to do by using that SUCURI_DATA_STORAGE constant. If I can use web/app/sucuri to store Sucuri logs/data (instead of the default web/app/uploads/sucuri), that would work much better for me, since then Sucuri logs/data will be isolated in the environments I plan on launching to.