ixis / codeception-module-drupal-user-registry

A Codeception module for managing test users on Drupal sites.
MIT License
5 stars 10 forks source link

escapeshellarg() on Windows can result in incorrect password #13

Open mcjim opened 9 years ago

mcjim commented 9 years ago

See codeception-module-drupal-user-registry/src/Drupal/UserRegistry/DrushTestUserManager.php

escapeshellarg($user->pass), where $user->pass is "abc!!!" will result in "abc ".

This, I think, is because the Windows version of escapeshellarg wraps the arg in double quotes. Manually executing the drush command with double quotes also fails (drush upwd name --password="abc!!!") but works with single quotes (drush upwd name --password='abc!!!').

So, I think some alternative method of escaping the password is needed. Windows does normally want the double quotes, so this is an exception to the rule.

ixisandyr commented 9 years ago

This is proving to be tricky:

On Windows, escapeshellarg() instead removes percent signs, replaces double quotes with spaces and adds double quotes around the string.

! is also affected from php 5.4 onwards https://bugs.php.net/bug.php?id=69768

So passwords with percent or double quote would not work either.

Looking into how how command line arguments are processed, it does all seem a bit broken. e.g.

x.php

<?php

var_dump($_SERVER['argv'][1]);

If my password was '%temp%'

Example 1) No quotes, no escaping. Treated as variable.

c:\php>php.exe x.php %temp%
string(32) "C:\Users\andy\AppData\Local\Temp"

Example 2) No quotes, escaped percent. Treated as literal, this is what we want.

c:\php>php.exe x.php ^%temp^%
string(6) "%temp%"

Example 3) With quotes and escaped. Includes the escape chars.

c:\php>php.exe x.php "^%temp^%"
string(8) "^%temp^%"

So, if my password was "my %temp% password" there is no way to pass this as an argument.

I guess this could be considered an edge case, but it shows it's difficult to fix for all cases.

Another approach is not to escape it at all on windows. It's 'user input' but does not come from an untrusted source. We could just do:

$arg = '"' . addcslashes($arg, '"') . '"';

But there are issues with special chars, such as >. For example a password of:

">a

If we pass that as an argument to the dir command it gets run as: dir "\">a" which will write the output of the dir command to file a!

The symfony/process component attempts to address the variable issue, but does not cater for exclamation marks:

https://github.com/symfony/process/blob/master/ProcessUtils.php (see escapeArgument())

I tried basing something of this in my fork, but I hit the same kind of issues. I'll revisit that and see if something drops out.

Otherwise, I'm not certain of the way forward here. Perhaps the easiest is to display an error if running on windows and any of the parameters that get passed to drush contain %, " or !

At least that way, the user won't be left scratching their head as to why the password does not work.