On Tue, Apr 18, 2023 at 11:25:11AM +0200, Corinna Vinschen via Cygwin wrote:
> Hi Bruno,
> 
> On Apr 17 20:44, Bruno Haible via Cygwin wrote:
> > Hi Corinna,

Jumping in to this conversation a bit belatedly, but as someone on the
Austin Group that can try to get an answer upstream...

> But I'm not *that* happy with the change yet, for two reasons.
> 
> First, the security risk outlined in
> https://sourceware.org/bugzilla/show_bug.cgi?id=13134#c0 doesn't
> actually exist on Cygwin, because we don't implement setuid/setgid
> executables.  You can set the mode bits, but they are not acted upon.

The glibc bug points to the sample posix_spawn() implementation in
POSIX XRAT - but that example implementation is non-normative and
known buggy, so it is not safe to rely on it.

Clarifying the wording in XRAT to explicitly mention that the example
is NOT bullet-proof (and that implementations should do better) is
probably worthwhile; I'll tackle that bug report.

> 
> Second, the rational section in POSIX explains posix_spawn and
> posix_spawnp, but it does *not* actually provide an example
> implementation of posix_spawnp, only of posix_spawn.

POSIX is silent as to whether posix_spawnp() has to fall back to 'sh'
on ENOEXEC failure.  The p suffix is indeed similar to execvp() (which
DOES require a fallback to sh), but it could also just mean a
PATH-search, and not the PATH-search-and-sh-fallback of execvp().  As
we now have implementations in the wild that differ in behavior, and
use security as a reason for the divergence, it is worth getting that
clarified in POSIX.  I'll file a bug against POSIX shortly, and reply
again once it is up.

My personal preference: sh fallback on ENOEXEC is useful in execvp(),
but a bear to get right (see
https://www.austingroupbugs.net/view.php?id=1645 where POSIX has a bug
in requiring argv[0] to be the script's filename, which breaks busybox
sh and is NOT what glibc does; meanwhile, musl intentionally does NOT
do the sh fallback), so NOT doing it in posix_spawnp() would be
reasonable; but we'll have to see what the rest of the Austin Group
says.

> 
> From the above bugzilla entry I take it that on glibc, both
> functions tried to run the shell if the executable isn't recognized
> (up to commits d96de9634a33 / 13adfa34aff).
> 
> However, on Cygwin, only posix_spawnp does that,but not posix_spawn.
> 
> In fact, I read the POSIX descriptions in terms of these functions quite
> thoroughly, and at no point I see it mentioned that posix_spawnp shall
> *not* work like exevlp/execvp.  The crucial difference between posix_spawn
> and posixc_spawnp is described in an interestingly vague way:
> 
>   posix_spawnp() interprets the second parameter more elaborately than
>   posix_spawn().
> 
> If I missed the point in the POSIX docs, please tell me.

Yeah, it appears that POSIX is (accidentally) silent on whether
posix_spawnp() has to do the sh fallback on ENOEXEC; but it seems
quite reasonable that posix_spawn() being more like execle() must NOT
do a sh fallback.

> 
> So, again, the patch is simple.  But it's kind of a pity that the change
> in glibc has been made without a bigger discussion.  Right now, it looks
> like the glibc change to posix_spawn was correct, but the change to
> posix_spawnp was arbitrary.
> 
> Has anybody attempted to ask the Austin group to define this behaviour
> in posix_spawnp more concise?  Is there a protocel from the Austin
> group?  If not, wouldn't it be time to ask the Austin group?

Doing that now ;)

> 
> > Btw, there are two more functions in the posix_spawn family meanwhile:
> >   * posix_spawn_file_actions_addchdir_np
> >     implemented by glibc [1], musl libc, macOS, FreeBSD [2], Solaris ≥ 11.3
> >     used by a few packages (Firefox, Chromium, Rust)
> >   * posix_spawn_file_actions_addfchdir_np
> >     implemented in glibc, musl libc
> >     but not used by any package so far [3].
> > 
> > The next POSIX will contain these functions (without the _np suffix).[4]
> 
> Thanks for the pointers.  I'm not sure I'll have the time to implement
> them soon, but I put them on my list for 3.5.0.  Patches welcome!
> 
> 
> Thanks,
> Corinna

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


-- 
Problem reports:      https://cygwin.com/problems.html
FAQ:                  https://cygwin.com/faq/
Documentation:        https://cygwin.com/docs.html
Unsubscribe info:     https://cygwin.com/ml/#unsubscribe-simple

Reply via email to