rocklobster-in / contact-form-7

Contact Form 7 - Just another contact form plugin for WordPress.
Other
287 stars 140 forks source link

wpcf7_is_file_path_in_content_dir has incompatibility with symbolic links #475

Closed jjpeleato closed 3 years ago

jjpeleato commented 3 years ago

Hello @takayukister

Current Behavior

Asset Version
WordPress 5.7.2
Contact Form 7 5.4.2

I have detected an incompatibility of the "wpcf7_is_file_path_in_content_dir" function on "includes/validation-functions.php" file when the server works with symbolic links.

When the symbolic links exist, $path and WP_CONTENT_DIR are not the same.

Look at the differences:

"/home/.../sites/.../current/shared/public/wp-content/uploads/wpcf7_uploads/1885601162/ReadMe.pdf" "/home/.../sites/.../current/releases/87/public/wp-content"

Expected Behavior

I think if checking if the file exists with the $path variable, the function is more simplified, or if you use the WPCF7_UPLOADS_TMP_DIR, the function must check this constant.

Possible Solution

function wpcf7_is_file_path_in_content_dir( $path ) {
    if ( 0 === strpos( realpath( $path ), realpath( WP_CONTENT_DIR ) ) ) {
        return true;
    }

    if ( defined( 'WPCF7_UPLOADS_TMP_DIR' ) 
        and 0 === strpos( realpath( $path ), realpath( WPCF7_UPLOADS_TMP_DIR ) ) ) {
        return true;
    }

    if ( defined( 'UPLOADS' )
        and 0 === strpos( realpath( $path ), realpath( ABSPATH . UPLOADS ) ) ) {
        return true;
    }

    return false;
}

Thank you, @jjpeleato

takayukister commented 3 years ago

The purpose of wpcf7_is_file_path_in_content_dir() is to check if the given file path is under the content directories that WordPress takes control.

When the symbolic links exist, $path and WP_CONTENT_DIR are not the same.

If $path and WP_CONTENT_DIR are not the same, wpcf7_is_file_path_in_content_dir() should return false, regardless of symlinks. That is the expected behavior.

I don't agree with your PR idea because WPCF7_UPLOADS_TMP_DIR is not provided for security restriction loopholes.

jjpeleato commented 3 years ago

Hi @takayukister,

I remove the word "bug" in the title. I understand your comment, but I am confused.

wpcf7_is_file_path_in_content_dir always return false when it checks the $path with symlinks.

In the following documentation:

https://contactform7.com/file-uploading-and-attachment/#How-your-uploaded-files-are-managed

It is possible to change the temp upload directory, but in the function compose on includes/mail.php (on line 139) always returns false even if the temp file exists.

Also if I change to another directory that is outside the wp-content path wpcf7_is_file_path_in_content_dir functions always return false.

How should this case work?

Thank you, @jjpeleato

takayukister commented 3 years ago

Even if WPCF7_UPLOADS_TMP_DIR is defined, the value should refer to a directory under WP_CONTENT_DIR. I'm thinking about displaying a warning in the admin screens when WPCF7_UPLOADS_TMP_DIR is set to outside WP_CONTENT_DIR. Security is the top priority on this matter.

m-ovs commented 5 months ago

I found this old issue by googling the same problem with symlink. And there is a solution better then adding a new constant. You can override tmp directory by the hook. But the question - why don't you make your own tmp directory in /wp-content outside /uploads?

function override_wpcf7_upload_tmp_dir($type)
    {
        $type = [
            'url' => $type['url'],
            'dir' => WP_CONTENT_DIR . '/tmp',
        ];

        return $type;
    }
add_filter('wpcf7_upload_dir', 'override_wpcf7_upload_tmp_dir');