search

deliciousbrains / wp-migrate-db

WordPress plugin that exports your database, does a find and replace on URLs and file paths, then allows you to save it to your computer.
https://wordpress.org/plugins/wp-migrate-db/
341 stars 515 forks source link

WP CLI migratedb command removes slashes from the output file name #140

Closed jormaster3k closed 1 year ago

jormaster3k commented 2 years ago

It appears the latest release version 2.3.3 has a regression bug in the WP CLI migratedb function that removes slashes from the output file name.

We have scripts that take the created SQL file and import it into a new database using mysql, but since the filename changed, our scripts can no longer find the file and are failing.

This is on a Ubuntu 18.04 Linux server, so we're using forward slashes in the path.

Steps To Reproduce

Use WP CLI migratedb command to export your wordpress site to a path containing forward slashes

sudo wp --allow-root migratedb export "/tmp/backup.sql"

Expected Results

The output filename is used verbatim

Observed Results

The output filename has trailing slashes removed:

Expected filename: /tmp/backup.sql Observed filename: tmpbackup.sql

myuser@myhost:/path/to/my/wordpress/site$ sudo wp --allow-root migratedb export "/tmp/backup.sql" Initiating migration... Exporting tables 100% [============================================================================] 0:01 / 0:01 Cleaning up... Success: Export saved to: tmpbackup.sql

I verified that this does not occur on 2.3.2, so something changed in the 2.3.3 release to break this.

jormaster3k commented 2 years ago

Looks like the change from trim() to sanitize_file_name() on class/Common/Cli/Command.php caused this.

https://github.com/deliciousbrains/wp-migrate-db/commit/b3ba13cc1c9a995867a6783f2eb9bf1cd7839d4c#diff-59f06d5efe617d863ce1d9a2b429d0160871099c299964654092f412eccc5c61

jormaster3k commented 1 year ago

Hi, just checking to see if anyone can look at this? This seems like a pretty cut-and-dry bug, and was hoping it was an easy fix.

philwp commented 1 year ago

@jormaster3k the slash removal was added as a security fix, as it was potentially possible to pass malicious code through the CLI. Perhaps you can cd into the directory within your script to export it somewhere else.

jcvanhalle-ec commented 1 year ago

@jormaster3k the slash removal was added as a security fix, as it was potentially possible to pass malicious code through the CLI. Perhaps you can cd into the directory within your script to export it somewhere else.

How would that work when running wp migrate remotely via SSH (--ssh) ? If the SSH user has a home the export seems to get dropped there but what if the SSH user has no writing rights or no home ? Losing the ability to pick up the destination path breaks down some poor-man's export workflow to local instances.

Should we just run the command over SSH instead of using the SSH flag ?

kevinwhoffman commented 1 year ago

Hi folks, just following up to let you know that WP Migrate 2.6 once again allows paths to be provided with slashes so that you can write the export to a directory that is different from the one in which you are running the command.

Regarding the security concerns, we have limited the sanitization to the final part of that path after the last slash, so the sanitization still takes place but no longer prevents you from providing a path to another directory.