Change Details

This stack allocation turns into a potentially exploitable heap overflow in the context of posix_spawnp(3) based solely on the PATH environment variable. On x86 only, posix_spawnp(3) will allocate a small 4k stack to work with since rfork(2) w/ RFMEM cannot function correctly there. The logic isn't too hairy if we instead rewrite the PATH handling to operate on the immutable string passed in, so do this instead of the arbitrary stack allocation. Additional size is also needed for the stack in posix_spawnp(), because it may need to push all of argv to the stack and rebuild the command with `sh` in front of it. We'll make sure it's properly aligned for the new thread, but future work should likely make rfork_thread a little easier to use by ensuring proper alignment. This has not made it into any final release, but this does appear as written in releng/11.4; tagging in: - kib and jilles as knowledgeable reviewers - Andrew Gierth, who reported this to me today, 2020/05/28 - #secteam to handle it for to the in-progress release cycle - #releng to keep them apprised I've limited visibility so that someone who isn't me can decide if it's worth being limited or not. =-)

Some environments in which execvPe may be called have a limited amount of stack available. Currently, it avoidably allocates a segment on the stack large enough to hold PATH so that it may be mutated and use `strsep()` for easy parsing. This logic is now rewritten to just operate on the immutable string passed in and do the necessary math to extract individual paths, since it will be copying out those segments to another buffer anyways and piecing them together with the name for a full path. Additional size is also needed for the stack in posix_spawnp(), because it may need to push all of argv to the stack and rebuild the command with `sh` in front of it. We'll make sure it's properly aligned for the new thread, but future work should likely make rfork_thread a little easier to use by ensuring proper alignment. Some trivial cleanup has been done with a couple of error writes, moving strings into char arrays for use with the less fragile sizeof().

This stack allocation turns into a potentially exploitable heap overflow in the context of posix_spawnp(3) based solely on the PATH environment variableSome environments in which execvPe may be called have a limited amount of stack available. Currently, it avoidably allocates a segment on the stack large enough to hold PATH so that it may be mutated and use `strsep()` for easy parsing. On x86 onlyThis logic is now rewritten to just operate on the immutable string passed in and do the necessary math to extract individual paths, posix_spawnp(3)since it will allocate a small 4k stack to work with since rfork(2) w/ RFMEM cannot function correctly therebe copying out those segments to another buffer anyways and piecing them together with the name for a full path. The logic isn't too hairy if we instead rewrite the PATH handling to operate on the immutable string passed in, so do this instead of the arbitrary stack allocation. Additional size is also needed for the stack in posix_spawnp(), because it may need to push all of argv to the stack and rebuild the command with `sh` in front of it. We'll make sure it's properly aligned for the new thread, but future work should likely make rfork_thread a little easier to use by ensuring proper alignment. This has not made it into any final release, but this does appear as written in releng/11.4; tagging in: - kib and jilles as knowledgeable reviewers - Andrew Gierth, who reported this to me today, 2020/05/28 - #secteam to handle it for to the in-progress release cycle - #releng to keep them apprised I've limited visibility so that someone who isn't me can decide if it's worth being limited or not.Some trivial cleanup has been done with a couple of error writes, =-)moving strings into char arrays for use with the less fragile sizeof().