tags:

views:

63

answers:

3

Given certain multibyte character sets, am I correct in assuming that the following doesn't do what it was intended to do?

$string = str_replace('"', '\\"', $string);

In particular, if the input was in a character set that might have a valid character like 0xbf5c, so an attacker can inject 0xbf22 to get 0xbf5c22, leaving a valid character followed by an unquoted double quote (").

Is there an easy way to mitigate this problem, or am I misunderstanding the issue in the first place?

(In my case, the string is going into the value attribute of an HTML input tag: echo 'input type="text" value="' . $string . '">';)

EDIT: For that matter, what about a function like preg_quote()? There's no charset argument for it, so it seems totally useless in this scenario. When you DON'T have the option of limiting charset to UTF-8 (yes, that'd be nice), it seems like you are really handicapped. What replace and quoting functions are available in that case?

+3  A: 

No, you’re right: Using a singlebyte string function on a multibyte string can cause an unexpected result. Use the multibyte string functions instead, for example mb_ereg_replace or mb_split:

$string = mb_ereg_replace('"', '\\"', $string);
$string = implode('\\"', mb_split('"', $string));

Edit    Here’s a mb_replace implementation using the split-join variant:

function mb_replace($search, $replace, $subject, &$count=0) {
    if (!is_array($search) && is_array($replace)) {
        return false;
    }
    if (is_array($subject)) {
        // call mb_replace for each single string in $subject
        foreach ($subject as &$string) {
            $string = &mb_replace($search, $replace, $string, $c);
            $count += $c;
        }
    } elseif (is_array($search)) {
        if (!is_array($replace)) {
            foreach ($search as &$string) {
                $subject = mb_replace($string, $replace, $subject, $c);
                $count += $c;
            }
        } else {
            $n = max(count($search), count($replace));
            while ($n--) {
                $subject = mb_replace(current($search), current($replace), $subject, $c);
                $count += $c;
                next($search);
                next($replace);
            }
        }
    } else {
        $parts = mb_split(preg_quote($search), $subject);
        $count = count($parts)-1;
        $subject = implode($replace, $parts);
    }
    return $subject;
}

As regards the combination of parameters, this function should behave like the singlebyte str_replace.

Gumbo
There is no mb_str_replace in PHP
reko_t
Your syntax for mb_ereg_replace() is wrong - it requires a regular expression. I was hoping to avoid heavier processing required by this kind of function, but I guess I'm out of luck. I need to use this for EVERYTHING - even something like preg_quote(), too, right? Despite the normal ereg_* functions being deprecated as of PHP5.3, does the same not apply to mb_ereg_* functions?
@user456885: `"` is a valid regular expression that describes the character `"` (note that this is not PCRE). But why don’t you use the second split-join variant?
Gumbo
Sorry, my bad on the PCRE. The split/join trick is nifty indeed. I guess I'm just holding out hope that I don't have to write custom replacements for such trivial things as a simple str_replace() or even standard, built-in PHP functions like preg_quote(). Bummer.
Also, I know the ereg_* functions are now deprecated - does that (or *will* that) include the mb_ereg_* functions too?
Nice work. As reko_t said, though, don't you still have to specify the charset with mb_regex_encoding() BEFORE you use either of these suggested solutions?
Also, watch out for that preg_quote() in your code (which is ALSO not multibyte safe). Presumably, $search is not external input, so in most cases it's OK, but this one could come back to bite you in rare situations.
@user456885: The [configured internal character encoding](http://php.net/manual/en/mbstring.configuration.php#ini.mbstring.internal-encoding) is used if not other specified.
Gumbo
+2  A: 

You could use either mb_ereg_replace by first specifying the charset with mb_regex_encoding(). Alternatively if you use UTF-8, you can use preg_replace with the u modifier.

reko_t
I can't limit to UTF-8 unfortunately, which I think would solve the problem. I guess mb_ereg_replace() is the only solution out there (?) ... but seems inefficient for a simple str_replace(). I'd have to call it as a replacement for preg_quote() too, eh? ... Also, I know the ereg_* functions are now deprecated - does that include the mb_ereg_* functions too?
A: 

From what I understand, much of this type of string injection is solved by the mysql_real_escape_string(); function.

http://php.net/manual/en/function.mysql-real-escape-string.php

jeffkee
That's a function that is tied to a particular database driver. I don't want to rely on some database driver's rules to escape data that's not even going to the database. I also don't want to require the use of a database driver if the code doesn't use a database in the first place.