Skip to content

Commit

Permalink
Fix #365. Mysql credentials leak.
Browse files Browse the repository at this point in the history
  • Loading branch information
weitzman committed Mar 31, 2014
1 parent 78f3501 commit a5ce745
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 24 deletions.
14 changes: 9 additions & 5 deletions commands/sql/sql.drush.inc
Expand Up @@ -69,7 +69,12 @@ function sql_drush_command() {
$items['sql-connect'] = array(
'description' => 'A string for connecting to the DB.',
'bootstrap' => DRUSH_BOOTSTRAP_DRUSH,
'options' => $options + $db_url,
'options' => $options + $db_url + array(
'extra' => array(
'description' => 'Add custom options to the mysql command.',
'example-value' => '--skip-column-names',
),
),
'examples' => array(
'`drush sql-connect` < example.sql' => 'Import sql statements from a file into the current database.',
),
Expand Down Expand Up @@ -215,13 +220,12 @@ function sql_drush_command() {
$items['sql-cli'] = array(
'description' => "Open a SQL command-line interface using Drupal's credentials.",
'bootstrap' => DRUSH_BOOTSTRAP_DRUSH,
'options' => array(
'A' => 'Skip reading table information. This gives a quicker start of mysql.',
) + $options + $db_url,
// 'options' => $options + $db_url,
'allow-additional-options' => array('sql-connect'),
'aliases' => array('sqlc'),
'examples' => array(
'drush sql-cli' => "Open a SQL command-line interface using Drupal's credentials.",
'drush sql-cli -A' => "Open a SQL command-line interface using Drupal's credentials and skip reading table information.",
'drush sql-cli --extra=-A' => "Open a SQL CLI and skip reading table information.",
),
'remote-tty' => TRUE,
);
Expand Down
12 changes: 8 additions & 4 deletions lib/Drush/Sql/SqlBase.php
Expand Up @@ -33,7 +33,7 @@ public function command() {}
* @return string
*/
public function connect() {
return trim($this->command() . ' ' . $this->creds() . ' ' . drush_get_option('extra', $this->query_extra));
return trim($this->command() . ' ' . $this->creds(FALSE) . ' ' . drush_get_option('extra', $this->query_extra));
}


Expand Down Expand Up @@ -161,8 +161,8 @@ public function query($query, $input_file = NULL, $silent = TRUE, $result_file =

$parts = array(
$this->command(),
$this->silent(),
$this->creds(),
$this->silent(),
drush_get_option('extra', $this->query_extra),
$this->query_file,
drush_escapeshellarg($input_file),
Expand Down Expand Up @@ -270,10 +270,14 @@ public function db_exists() {}
public function delete() {}

/**
* Build a fragment containing credentials and other connection parameters.
* Build a fragment connection parameters.
*
* @param bool $hide_password
* If TRUE, DBMS should try to hide password from process list.
* On mysql, that means using --defaults-extra-file to supply the password.
* @return string
*/
public function creds() {}
public function creds($hide_password = TRUE) {}

/**
* The active database driver.
Expand Down
32 changes: 18 additions & 14 deletions lib/Drush/Sql/Sqlmysql.php
Expand Up @@ -5,14 +5,22 @@
class Sqlmysql extends SqlBase {

public function command() {
$command = 'mysql';
if (drush_get_option('A', FALSE)) {
$command .= ' -A';
}
return $command;
return 'mysql';
}

public function creds() {
public function creds($hide_password = TRUE) {
// EMPTY password is not the same as NO password, and is valid.
if (isset($this->db_spec['password'])) {
if ($hide_password) {
$data = "#This file was written by Drush.\n[client]\npassword=\"" . $this->db_spec['password'] . '"';
$file = drush_save_data_to_temp_file($data);
$parameters['defaults-extra-file'] = $file;
}
else {
$parameters['password'] = $this->db_spec['password'];
}
}

// Some drush commands (e.g. site-install) want to connect to the
// server, but not the database. Connect to the built-in database.
$parameters['database'] = empty($this->db_spec['database']) ? 'information_schema' : $this->db_spec['database'];
Expand All @@ -33,11 +41,6 @@ public function creds() {
// User is required. Drupal calls it 'username'. MySQL calls it 'user'.
$parameters['user'] = $this->db_spec['username'];

// EMPTY password is not the same as NO password, and is valid.
if (isset($this->db_spec['password'])) {
$parameters['password'] = $this->db_spec['password'];
}

return $this->params_to_options($parameters);
}

Expand Down Expand Up @@ -80,14 +83,15 @@ public function dumpCmd($table_selection, $file) {
// @see drush_get_option_help() in drush.inc
$ordered_dump = drush_get_option('ordered-dump');

$exec = 'mysqldump';
$exec = 'mysqldump ';
// mysqldump wants 'databasename' instead of 'database=databasename' for no good reason.
$exec .= str_replace('--database=', ' ', $this->creds());
if ($file) {
$exec .= ' --result-file '. $file;
}
// mysqldump wants 'databasename' instead of 'database=databasename' for no good reason.
// We had --skip-add-locks here for a while to help people with insufficient permissions,
// but removed it because it slows down the import a lot. See http://drupal.org/node/1283978
$extra = ' --no-autocommit --single-transaction --opt -Q' . str_replace('--database=', ' ', $this->creds());
$extra = ' --no-autocommit --single-transaction --opt -Q';
if (isset($data_only)) {
$extra .= ' --no-create-info';
}
Expand Down
2 changes: 1 addition & 1 deletion tests/sqlConnectTest.php
Expand Up @@ -27,7 +27,7 @@ function testSqlConnect() {
$shell_options = "-e";
$db_driver = $this->db_driver();
if ($db_driver == 'mysql') {
$this->assertRegExp('/^mysql --database=[^\s]+ --host=[^\s]+ --user=[^\s]+ --password=.*$/', $output);
$this->assertRegExp('/^mysql --password=.* --database=[^\s]+ --host=[^\s]+ --user=[^\s]+$/', $output);
}
elseif ($db_driver == 'sqlite') {
$this->assertContains('sqlite3', $output);
Expand Down

0 comments on commit a5ce745

Please sign in to comment.