Drush does not correctly shell-escape certain special characters (e.g. in mysql db passwords)

Seems reasonable.

Attached patch accomplishes the same by changing our existing escapeshellcmd to drush_escapeshellarg.

Not sure if it was because of this, but this is actually fixed in the code, latest -dev or git checkout does it, so mark it as fixed.

#2 was never committed, but by some coincidence I applied the same correction when I committed #766080: Windows support for drush: escaping the path to drush in backend invoke and elsewhere, so this issue is in fact fixed. Thanks for noticing.

I have the same problem with drush version 4.5
Looks like the patch wasn't applied into: /usr/local/Cellar/drush/4.5/commands/sql/sql.drush.inc
Still is:

  foreach ($parameters as $key => $value) {
    // Only escape the values, not the keys or the rest of the string.
    $value = escapeshellcmd($value);

Created separate bug: #1348110: drush sql-connect returns ERROR 1045 (28000): Access denied

Sorry, maybe I'm missing something here, but this bug still exists in 4.5, right?

I can confirm that the patch in #2 works for me

The metadata on the issue was wrong. It was fixed in drush-5, but not backported to drush-4. Most of the escape-related fixes done in drush-5 for Windows support were not backported, but Mark can decide if this should be an exception or not.

Ok thanks. upon more investigation I found that this patch did more harm than good. i will try the 5 branch.

For future benefits, I found that when using this syntax: $(drush @whatever sql-connect) < myfile.sql; with database passwords that have a ! in it, the exclamation point doesn't get escaped, so i piped it through sed like so:

$(drush $whatever sql-connect | sed 's/\!/\\\!/g') < file.sql

Since the patch in #2 is already in drush5, I'm committing that to 4.x.

I'm experiencing this issue with Drush 5.8 and PHP 5.3.10 on Linux. I've confirmed that the code from the patch exists in Drush.

sql-connect works fine for passwords lacking non-alphanumeric characters but the !%&' characters (these are the only ones I've tested) all result in a mysql 1045 (access denied) error.

Was there a regression or is this possibly an issue with PHP 5.3?

I'm guessing that spaces in passwords still work, and certain special characters never worked. Could you show the output of a run with the --debug or --simulate option?

tangent@machine:www $ drush @dev1.d7 sql-connect --debug
Bootstrap to phase 0. [0 sec, 3.04 MB]               [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drush() [0.01 sec, 3.21 MB] [bootstrap]
Loading drushrc "/home/tangent/.drush/drushrc.php" into "home.drush" scope. [0.01 sec, 3.22 MB]       [bootstrap]
Loaded alias @dev1.d7 from file /home/tangent/.drush/dev1.aliases.drushrc.php [0.02 sec, 3.23 MB]      [notice]
Cache HIT cid: 5.8-commandfiles-0-e5d59e584500a19f25a56aad00f0d73f [0.03 sec, 3.24 MB]               [debug]
Bootstrap to phase 0. [0.07 sec, 7.37 MB]                                               [bootstrap]
Bootstrap to phase 0. [0.09 sec, 7.38 MB]                                               [bootstrap]
Found command: sql-connect (commandfile=sql) [0.09 sec, 7.38 MB]       [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drupal_root() [0.12 sec, 7.43 MB]              [bootstrap]
Initialized Drupal 7.19 root directory at /var/www/drupal/drupal7/www [0.14 sec, 10.02 MB]                  [notice]
Drush bootstrap phase : _drush_bootstrap_drupal_site() [0.15 sec, 10.03 MB]             [bootstrap]
Initialized Drupal site www.canadascapital.gc.ca at sites/canadascapital.gc.ca [0.15 sec, 10.03 MB]          [notice]
Cache HIT cid: 5.8-commandfiles-2-eb40841279c55f5b8097ec798c21f6e1 [0.15 sec, 10.03 MB]              [debug]
Drush bootstrap phase : _drush_bootstrap_drupal_configuration() [0.16 sec, 11.52 MB]            [bootstrap]
Cache HIT cid: 5.8-commandfiles-3-678557a8132ba538e92b4f9751f3db24 [0.16 sec, 11.53 MB]             [debug]
mysql --database=capcan_drupal7 --host=localhost --user=test --password='x!%x'
Command dispatch complete [0.17 sec, 11.48 MB]               [notice]
 Timer  Cum (sec)  Count  Avg (msec) 
 page   0.006      1      5.81       

Peak memory usage was 11.6 MB [0.17 sec, 11.48 MB]            [memory]

tangent@machine:www $ drush @dev1.d7 sql-connect --simulate
mysql --database=capcan_drupal7 --host=localhost --user=test --password='x!%x'

tangent@machine:www $ `drush @dev1.d7 sql-connect`
ERROR 1045 (28000): Access denied for user 'test'@'localhost' (using password: YES)

tangent@machine:www $ mysql --database=capcan_drupal7 --host=localhost --user=test --password='x!%x'
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 4161
Server version: 5.5.29-0ubuntu0.12.10.1 (Ubuntu)

Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved.

Oracle is a registered trademark of Oracle Corporation and/or its
affiliates. Other names may be trademarks of their respective
owners.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

mysql> 

Looks like the shell escaping is okay, but perhaps MySQL expects its parameter to contain additional escaping when there are special characters. If #671906: mysql credentials leak in drush sqlc were fixed, that would take care of this issue as well.

Closed #1737054: sql-connect and sql-cli don't work if password contains a + as a dup of this issue.

This issue was marked closed (won't fix) because Drush has moved to Github.

If desired, you may copy this bug to our Github project and then post a link here to the new issue. Please also change the status of this issue to closed (duplicate).

Please ask support questions on Drupal Answers.

I've recreated this issue at https://github.com/drush-ops/drush/issues/125.