bitwiseworks / libc

LIBC Next (kLIBC fork)
9 stars 4 forks source link

spawnve may truncate environment #100

Closed dmik closed 3 years ago

dmik commented 3 years ago

I faced a really tricky flaw when working on https://github.com/bitwiseworks/git-os2/issues/4#issuecomment-808240492. Passing an envp array where some entry is an empty string (a valid pointer to a memory location which contains just \0) completely hides the rest of the environment from DosExecPgm.

This happens because the environment is passed to DosExecPgm as a single string (a concatenation of zero-terminated C strings followed by a zero byte) rather than as an array of strings. So, given this envp array

const char *envp[] = {
"VAR1=VAL1",
"VAR2=VAL2",
"",
"VAR3=VAL3",
NULL
}

the string for the DosExecPgm environment argument becomes this:

VAR1=VAL1\0VAR2=VAL2\0\0VAR3=VAL3\0\0

I.e. double zero comes before VAR3 and is interpreted as the end of the environment block so VAR3 remains completely unseen by DosExecPgm and does not appear in the child process.

dmik commented 3 years ago

Note that there is also another related bug exposing a security risk (and arbitrary memory access). If envp is an array containing just a single NULL element, then the standard environ array in the child process will contain garbage (instead of just containing a single NULL element). A modified test case from #102 will demonstrate that if you change

    char * env [] = { "ENV1", "ENV2=VAL2", NULL };

with just

    char * env [] = {  NULL };

Note that this results in an environment argument for DosExecPgm being a string containing a single zero character and DosExecPgm itself is happy with that. It will leave it like this in the child process information block as well. So the guilty part here is spawnve which performs environ to string conversion.

dmik commented 3 years ago

Note that we should only fix spawnve and __spawnve because all other variants call spawnve which in turn calls its work horse, __spawnve.

dmik commented 3 years ago

Note that commit c37ab79 fixes the startup code composing environ as this is where the second problem was lying.