interconnectit / Search-Replace-DB

This script was made to aid the process of migrating PHP and MySQL based websites. Works with most common CMSes.
https://interconnectit.com/products/search-and-replace-for-wordpress-databases/
GNU General Public License v3.0
3.99k stars 850 forks source link

Possible XSS vulnerability #359

Closed enferas closed 3 years ago

enferas commented 3 years ago

Hello,

I would like to report for a possible XSS vulnerabilities through the input 'search'.

In the file index.php line 142.

 $this->search  = isset( $_POST['search'] ) ? $_POST['search'] : '';

The the parent class will be called in line 245.

$parent = parent::__construct( array(
                    'name'         => $this->name,
                    'user'         => $this->user,
                    'pass'         => $this->pass,
                    'host'         => $this->host,
                    'port'         => $this->port,
                    'search'       => $this->search,
                    'replace'      => $this->replace,
                    'tables'       => $this->tables,
                    'dry_run'      => $this->dry_run,
                    'regex'        => $this->regex,
                    'exclude_cols' => $this->exclude_cols,
                    'include_cols' => $this->include_cols
                ) );

Inside the file srdb.class.php line 325.

$report = $this->replacer( $this->search, $this->replace, $this->tables, $this->exclude_tables );

Inside the file srdb.class.php line 991.

$this->log( 'search_replace_table_start', $table, $search, $replace );

Inside the file srdb.class.php

// line 239
$args = array_slice( func_get_args(), 1 );
// line 248
list( $table, $search, $replace ) = $args;
// line 257
$output .= "{$table}: replacing {$search} with {$replace}";
//line 298
echo $output . "\n";
gianluigi-icit commented 3 years ago

Thanks a lot for take time and open this issue!

That's true, you might leak some information with this tool and even attempt a XSS, it's easy to get troubles. That's why we recommenced to delete the script as soon as you are done with it. People used to upload it online and forget about it, that's why we removed the ability to read wp-config files!

I might start to sanitize all the inputs, write the tests for the passwords, check if everything it's fine. This requires time and we work on this tool in our spare time.

Would you like to write a patch for it?

edit spoiler alert: this message is not a proof

enferas commented 3 years ago

Thank you for your proof.

I am not sure if I can write the patch. But I can recommend to sanitize the three different sources in file index.php

$this->search  = isset( $_POST['search'] ) ? $_POST['search'] : '';
$this->replace = isset( $_POST['replace'] ) ? $_POST['replace'] : '';
$this->tables = isset( $_POST['tables'] ) && is_array( $_POST['tables'] ) ? $_POST['tables'] : array();

For two different sinks in file srdb.class.php

//line 298
echo $output . "\n";

//line 409
print_r( $args );

I would like to apply for CVE for my discovery and I would like use this issue as a reference.

gianluigi-icit commented 3 years ago

I suggest you to write the patch, you'll get how the script actually works. The line 409 is in the log function, its visibility is public and it's actually meant to be overridden by the cli class and write a more cleaner output at the line 289.

In any case, the web interface doesn't set the verbose property as true, hence it's not possible to trigger a XSS attack in that way as far as I know. Could you write few lines and prove the vulnerability?

gianluigi-icit commented 3 years ago

I like the idea for CVE, but you have to get your hands dirty and actually prove the vulnerability, I really spent few minutes to wrap my head around the code and check those lines.

gianluigi-icit commented 3 years ago

no feedback so far, closing.