views:

511

answers:

2

I have a PHP page that allows people to run htpasswd to update their password. What is the best way to sanitize their input. I don't want to restrict the input to much because I want to allow for secure passwords. This is what I have. How can it be improved?

$newPasswd = preg_replace('/[^a-z0-9~!()_+=[]{}<>.\\\/?:@#$%^&*]/is', '', $inputPasswd);
$cmdline = $htpasswd . " " . $passwd_file . " " . escapeshellarg($username) . " " .escapeshellarg($newpasswd);
exec( $cmdline, $output, $return_var );
+1  A: 

I would not use preg_replace with empty string on "dangerous characters", since that will make the user believe he changed the password into something, but you exec something else.. So the user will set the password to something different than he put in... It would be better to give feedback to the user telling him that some characters (preferably capture and display the offending characters) did not pass through validation.

Other than that, I think it looks pretty good. I think it provides the user with a wide range of possible character, and he doesn't need more than those to create a secure password. It might also be useful to tell the user which characters that are allowed in the password so he doesn't have to guess.

PatrikAkerstrand
This is actually part of a plug in, and I have limited ability to interact with the user. The likelyhood of a real user entering bad characters is low enough that I am willing to deal with the support calls.
Jon Drnek
Although, I could probably cause the password change to fail earlier... I'll look into that
Jon Drnek
+6  A: 

The ecscapeshellarg function is built-in to PHP, and differs across systems based on the shell to sanitize the text so that no "unsafe" characters can be passed into exec.

http://us2.php.net/manual/en/function.escapeshellarg.php

MiffTheFox
Are you saying that I don't need to bother removing bad characters if I use escapeshellarg?
Jon Drnek
Escapeshellarg will strip out anything that could be interpreted by the shell. (Or change it, for example ' => \') I'm not that familiar with htpasswd, but this would prevent any shell injection attacks.
MiffTheFox