How to stop escapeshellarg() from turning "s into blank spaces on Windows?

Question

The PHP function escapeshellarg() turns this:

"A schema"."A table"

Into this:

" A schema . A table "

That is, it replaces the quotes with spaces instead of quoting them. The expected and desired result (by me) is:

"\"A schema\".\"A table\""

This has wasted countless hours of my life troubleshooting. For the longest time, I just couldn't figure out what was happening; it never occured to my brain that it could be so stupid as to actually remove the double quotes entirely! Even if this "makes it safe", it's such a bizarre thing to do when escaping them would not only be the standard and supported way of handling this, but would result in no problems arising from using this function.

It should be noted that I've read the comments on: https://www.php.net/escapeshellarg

I have also read a lot of other texts complaining about this function and about Windows shell escaping and whatnot, so I'm at least "vaguely" aware of the reason: "It's complicated."

However, I find myself utterly crippled by this core function (escapeshellarg), as sadly so often is the case with PHP, and I find myself trying to make my own function to replace it. This, of course, is extremely problematic and scary since half the point of this function is security-related! It's supposed to "make safe" an arbitrary string to be used as an argument on the command line, without risking that an attacker can "break out" from it and execute arbitrary commands!

For this reason, I'm pretty sure that simply doing:

$likely_not_a_safe_argument = '\"' . str_replace('"', '\"', $user_input) . '\"';

... is NOT safe! If it were that simple, then I'm certain that the PHP developers would do that for Windows inside escapeshellarg.

Basically, I have no idea what to do now. And yes, I do need to send double quotes in arguments. Here is a real-world example of where it's necessary:

pg_dump --format plain --verbose --file "dump.txt" --exclude-table-data="\"Test schema\".\"Test table\"" --host="localhost" --port="5432" --username="postgres" --dbname="testdb"

(No, it doesn't take single quotes or anything else. It must be double quotes around the identifiers.)

How should I safely "improve" escapeshellarg() so that it doesn't nuke my double quotes? What does everyone else do?

Answer 1

you could use the addslashes(); function.

https://www.php.net/manual/de/function.addslashes.php

Answer 2

first off, PHP's windows' escapeshellarg() function is not good, in fact it's bad, it fails to escape perfectly valid arguments like ÆØÅ, sometimes corrupting the input in the process. you could probably create a proper escapeshellarg_windows() function by reversing ReactOS's CommandLineToArgvW()-implementation here: https://github.com/reactos/reactos/blob/10d1afea18db0ef06942239867cbc090e2330b90/dll/win32/shell32/wine/shell32_main.c#L79

still, i think this would work for quotes, and be practically equally safe:

function escapeshellarg_shitty_windows_quote_hack(string $arg): string
{
    $hack = bin2hex(random_bytes(11));
    $arg = strtr($arg, [
        '"' => $hack
    ]);
    $arg = escapeshellarg($arg);
    $arg = strtr($arg, [
        $hack => '"""'
    ]);
    return $arg;
}

basically just replacing " with """ , which seems to be an escape sequence for quotes inside quotes, per ReactOS's, CommandLineToArgvW() code

            /* Now count the number of consecutive quotes. Note that qcount
             * already takes into account the opening quote if any, as well as
             * the quote that lead us here.
             */
            while (*s=='"')
            {
                if (++qcount==3)
                {
                    *d++='"';
                    qcount=0;
                }
                s++;
            }

but note that this is still not a good escapeshellarg function for windows. if that's actually needed, i would probably try to reverse the effects of ReactOS CommandLineToArgvW() function, in PHP

(i'm far too tired to attempt that now though, but even if i wasn't, i'd probably feel safer if som1 else did it)

• as for the use of an 88 bits entropy placeholder, see https://crypto.stackexchange.com/a/13305/29537 - 88 bits should be far more than necessary to ensure uniqueness