SMF Development > Bug Reports
Repair Settings can't handle " in the DB password
Arantor:
Given that this is dealing with Settings.php, we don't need to be nearly as generous as some of the conditions you've outlined... there are no variables with names with digits in, there should be no leading spaces etc. - no need to worry about them.
MrPhil:
@Arantor -- can you give an example where the db_password fails with a " in it? What are the string delimiters... ' or "? Is the " escaped \" (mandatory if the string delimiters are ")? Is it PHP that's throwing an error, or MySQL (that it's received a bogus password)? stripslashes() appears to be applied to everything. Is it only the db password that has problems, or does any string containing a ' or " (escaped or not) have a problem? Don't forget that when repair_settings.php rewrites the Settings.php file, it always uses ' as the string delimiter. I added a call to addslashes() for the string data -- maybe that's needed?
I looked through the code and the regexp, and I see that an escape \ will be ignored, which is bad news if the escaped character is the same as the delimiters. I had to write some code to go through the string and skip over escaped characters, so it would know where the string really ended. Since a value string might have both ' and " in it, we just can't scold users to remember to always use the "other" character as the delimiter (and the rewrite is '). We have the delimiter available in $match, so the rewrite of lines could use the original delimiter.
@emanuele, can you describe an example of ; breaking a password? What happens to ; in other strings?
In repair_settings.php, find
--- Code: --- if (substr($settingsArray[$i], 0, 1) == '$')
{
preg_match('~^[$]([a-zA-Z_]+)\s*=\s*(["\'])?(.*?)(?:\\2)?;~', $settingsArray[$i], $match);
if (isset($match[3]))
{
if ($match[3] == 'dirname(__FILE__)')
$settings[$match[1]] = dirname(__FILE__);
elseif ($match[3] == 'dirname(__FILE__) . \'/Sources\'')
$settings[$match[1]] = dirname(__FILE__) . '/Sources';
elseif ($match[3] == '$boarddir . \'/Sources\'')
$settings[$match[1]] = $settings['boarddir'] . '/Sources';
else
$settings[$match[1]] = stripslashes($match[3]);
}
}
--- End code ---
and change it to
--- Code: --- if (preg_match('~^\s*\$~', $settingsArray[$i])) // allow ws before $ in var name
{
preg_match('~^(\s*)\$([a-zA-Z_]\w*)\s*=\s*(["\'])?(.*)~', $settingsArray[$i], $match);
// match[1] is any leading whitespace, if you need it
// process match[4] to find its end, and split off remainder to match[5]
$string = $match[4];
for ($j=0; $j<strlen($string); $j++) {
$char = substr($string, $j, 1);
if ($char == '\\') {
$j++; // skip escaped character, too
continue;
}
if ($char == $match[3]) { // is non-empty delimiter (and not escaped)?
$match[4] = substr($string, 0, $j); // truncate string here
//$match[5] = substr($string, $j+1); // remainder including ; to element 5
break;
}
if ($match[3] == '' && $char == ';') { // no string delimiter, ran into semi
$match[4] = substr($string, 0, $j); // truncate non-string here at ;
//$match[5] = substr($string, $j); // remainder including ; to element 5
}
}
$match[4] = trim($match[4]);
//$match[5] = trim($match[5]);
if (isset($match[4])) // if not set, it's a failure mode... just keeps from blowing up PHP
{
if ($match[4] == 'dirname(__FILE__)')
$settings[$match[2]] = dirname(__FILE__);
elseif ($match[4] == 'dirname(__FILE__) . \'/Sources\'')
$settings[$match[2]] = dirname(__FILE__) . '/Sources';
elseif ($match[4] == '$boarddir . \'/Sources\'')
$settings[$match[2]] = $settings['boarddir'] . '/Sources';
else
$settings[$match[2]] = stripslashes($match[4]);
}
}
--- End code ---
Find
--- Code: --- if (substr($settingsArray[$i], 0, 1) == '$' && preg_match('~^[$]([a-zA-Z_]+)\s*=\s*(["\'])?(.*?)(?:\\2)?;~', $settingsArray[$i], $match) == 1)
$settings[$match[1]] = stripslashes($match[3]);
foreach ($file_updates as $var => $val)
if (strncasecmp($settingsArray[$i], '$' . $var, 1 + strlen($var)) == 0)
{
$comment = strstr($settingsArray[$i], '#');
$settingsArray[$i] = '$' . $var . ' = \'' . $val . '\';' . ($comment != '' ? "\t\t" . $comment : '');
}
--- End code ---
(Why is strncasecmp() used? That makes it case-insensitive, which doesn't seem to make sense.)
and change to
--- Code: --- if (preg_match('~^(\s*)\$([a-zA-Z_]\w*)\s*=\s*(["\'])?(.*)~', $settingsArray[$i], $match)) {
// match[1] is any leading whitespace, if you need it
// process match[4] to find its end, and split off remainder to match[5]
$string = $match[4];
for ($j=0; $j<strlen($string); $j++) {
$char = substr($string, $j, 1);
if ($char == '\\') {
$j++; // skip escaped character, too
continue;
}
if ($char == $match[3]) { // is non-empty delimiter (and not escaped)?
$match[4] = substr($string, 0, $j); // truncate string here
//$match[5] = substr($string, $j+1); // remainder including ; to element 5
break;
}
if ($match[3] == '' && $char == ';') { // no string delimiter, ran into semi
$match[4] = substr($string, 0, $j); // truncate non-string here at ;
//$match[5] = substr($string, $j); // remainder including ; to element 5
}
}
$match[4] = trim($match[4]);
//$match[5] = trim($match[5]);
$settings[$match[2]] = stripslashes($match[4]);
}
foreach ($file_updates as $var => $val)
// compare just up through variable name. was strncasecmp()... why case insensitive?
if (preg_match('~^\s*\$'. $var .'~', $settingsArray[$i]))
{
// rewrite line in a standard format, and add slashes to string values
$val = str_replace('\\', '\\\\', $val);
$val = str_replace("'", "\\'", $val);
$comment = strstr($settingsArray[$i], '#');
$settingsArray[$i] = '$' . $var . ' = \'' . $val . '\';' . ($comment != '' ? "\t\t" . $comment : '');
}
--- End code ---
This has not been more than very lightly tested (just segments in a test frame, not the whole thing end to end), so play with it for a while before turning it loose on a real live system! As with the original code, it assumes that the contents of Settings.php is legitimate PHP code, and that any syntax errors have already be caught and corrected (i.e., not extremely robust!). I've added a little flexibility in the ability to handle leading whitespaces and variable names with digits.
I didn't do anything (yet) about emptying out the file before rewriting it. That's a horrendous bug in the updating of Settings.php for database errors, and might cause problems here too.
Mod: replaced addslashes() in rewriting with str_replace() to only escape ' and \. Output strings should always be ' delimited.
Mod 2: do \ ->\\ before ' -> \'
Arantor:
--- Quote ---@Arantor -- can you give an example where the db_password fails with a " in it?
--- End quote ---
I thought I'd already explained. I started out setting the password manually myself by editing Settings.php, and needed repair_settings.php only to fix the paths in the database, namely the ones in the settings and themes tables.
The password is inserted into the first page of repair_settings.php as is, meaning it gets truncated (and on saving, everything fails because now the DB password is wrong.
If the correct password is then supplied, it is saved into the file with a \ in it, which in that case is wrong, it should not be so, because it is in single quotes, meaning that the password supplied to the DB is no longer blah"blah but now blah\"blah - no escape character should be inserted into single-quote strings unless the character being escaped is a ' or a \ itself - it is always treated as a literal in there.
I didn't look at any other string, as it was only the DB password field that had a " in it.
emanuele:
The root cause of the " breaking the password is simply because the html would look like this:
--- Code: ---<input value="passw"ord" .... />
--- End code ---
The root cause of the ; breaking the password is that the regexp tries to read the line and extracts the password stopping at the first ; that it finds (since ; is the end of line delimiter in php). So the solution I tried to implement in the regexp is: do the same thing, but it doesn't work...I think I'll simply change it to:
--- Code: ---^[$]([a-zA-Z_]+)\s*=\s*(["\'])?(.*?)(?:\\2)?;$
--- End code ---
Arantor:
--- Quote ---The root cause of the " breaking the password is simply because the html would look like this:
--- End quote ---
That's certainly why it truncates it, but that's not why it also proceeds to try escaping said " later on when actually saving.
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version