Re: Posix issue 8 pending change to system().
good point. fair enough, i'm convinced... fixed by adding "--" to system() and popen() in bionic, with corresponding new tests: https://android-review.googlesource.com/c/platform/bionic/+/1881791 On Fri, Oct 22, 2021 at 1:38 AM Geoff Clare wrote: > Rob Landley wrote, on 22 Oct 2021: > > > > On 10/21/21 6:38 PM, enh wrote: > > > > > it's unclear whether anyone's actually hit this in practice? and > even if they > > > had, their portable workaround would be to prefix with "exec "? > > > > > > maybe try libc-co...@lists.openwall.com and see if there's any > consensus that > > > this is worth the potential trouble? i'm worried that someone is > > > _deliberately_ using this to pass extra flags to the shell, which > wouldn't > > > have a workaround if we did make this change :-( > > It's not possible to pass "extra" flags when the "--" is not there, > because system() only passes one argument after the "-c" to sh. > So you can pass flags _instead_of_ a command string, but not _extra_ > flags. Doing this will just get you an error message from sh about > the command string argument being missing (except on a system were sh > accepts -c with no command string as an extension, but I doubt any > such system exists). > > > > > Pinging you and Rich was my attempt at that, but if there's a dedicated > list... > > Note that I removed the libc-coord list from the Cc. > > -- > Geoff Clare > The Open Group, Apex Plaza, Forbury Road, Reading, RG1 1AX, England >
Re: Posix issue 8 pending change to system().
On 2021-10-23 22:57:09 -0500, Rob Landley via austin-group-l at The Open Group
wrote:
> On 10/23/21 8:39 PM, Vincent Lefevre wrote:
> > On 2021-10-23 18:08:37 -0500, Rob Landley via austin-group-l at The Open
> > Group wrote:
> >> Testing:
> >>
> >> $ bash -c '-i echo hello'
> >> bash: - : invalid option
> >> $ dash -c '-i echo hello'
> >> dash: 0: Illegal option -
> >> # android
> >> $ sh -c "-i echo hello"
> >> sh: sh: - : unknown option
> >>
> >> So implementing system with arguments {"sh", "-c", "--", cmd, 0}
> >> shouldn't break any existing use case that worked before.
> >
> > Couldn't this potentially introduce a security issue, in case the
> > system() argument would come from untrusted data, with sanitization
> > assuming "sh -c"?
> >
> > I mean that some strings would be expected to fail, but if "--" is
> > introduced, real code could be executed.
>
> You're suggesting that if we explicitly fix the reported bug, and have no side
> effects other than fixing that bug, this is potentially a bad thing.
>
> You're suggesting "I'm going to pass untrusted strings to sh -c and maybe bash
> will error out instead of running some of them" can be feasibly described as a
> security strategy.
No, this is not what I've said. On the contrary, I've said that
there is *sanitization* of untrusted data. My point is that this
sanitization could be based on the fact that the data are passed
to "sh -c" only, as currently specified, and code that is known
to fail could be passed as trusted (trusted, just because it is
known to fail, thus with no unexpected behavior).
--
Vincent Lefèvre - Web:
Re: Posix issue 8 pending change to system().
On 10/23/21 8:39 PM, Vincent Lefevre wrote:
> On 2021-10-23 18:08:37 -0500, Rob Landley via austin-group-l at The Open
> Group wrote:
>> Testing:
>>
>> $ bash -c '-i echo hello'
>> bash: - : invalid option
>> $ dash -c '-i echo hello'
>> dash: 0: Illegal option -
>> # android
>> $ sh -c "-i echo hello"
>> sh: sh: - : unknown option
>>
>> So implementing system with arguments {"sh", "-c", "--", cmd, 0}
>> shouldn't break any existing use case that worked before.
>
> Couldn't this potentially introduce a security issue, in case the
> system() argument would come from untrusted data, with sanitization
> assuming "sh -c"?
>
> I mean that some strings would be expected to fail, but if "--" is
> introduced, real code could be executed.
You're suggesting that if we explicitly fix the reported bug, and have no side
effects other than fixing that bug, this is potentially a bad thing.
You're suggesting "I'm going to pass untrusted strings to sh -c and maybe bash
will error out instead of running some of them" can be feasibly described as a
security strategy.
As Vetinari would say, "this is certainly a point of view".
> I'd say that this is very unlikely, but I'm wondering...
I would also consider it unlikely, yes.
Rob
P.S. It's also slated for issue 8, not yet another in-situ replacement for issue
7. Remember when locales showed up and "sort" no longer put things in ascii
order anymore and builds broke everywhere until we figured everything needed
LC_ALL=C ? Going susv4->susv5 is not guaranteed to have _zero_ impact.
Re: Posix issue 8 pending change to system().
On 2021-10-23 18:08:37 -0500, Rob Landley via austin-group-l at The Open Group
wrote:
> Testing:
>
> $ bash -c '-i echo hello'
> bash: - : invalid option
> $ dash -c '-i echo hello'
> dash: 0: Illegal option -
> # android
> $ sh -c "-i echo hello"
> sh: sh: - : unknown option
>
> So implementing system with arguments {"sh", "-c", "--", cmd, 0}
> shouldn't break any existing use case that worked before.
Couldn't this potentially introduce a security issue, in case the
system() argument would come from untrusted data, with sanitization
assuming "sh -c"?
I mean that some strings would be expected to fail, but if "--" is
introduced, real code could be executed.
I'd say that this is very unlikely, but I'm wondering...
--
Vincent Lefèvre - Web:
Re: Posix issue 8 pending change to system().
On 10/22/21 3:38 AM, Geoff Clare via austin-group-l at The Open Group wrote:
>> > maybe try libc-co...@lists.openwall.com and see if there's any consensus
>> > that
>> > this is worth the potential trouble? i'm worried that someone is
>> > _deliberately_ using this to pass extra flags to the shell, which wouldn't
>> > have a workaround if we did make this change :-(
>
> It's not possible to pass "extra" flags when the "--" is not there,
> because system() only passes one argument after the "-c" to sh.
> So you can pass flags _instead_of_ a command string, but not _extra_
> flags. Doing this will just get you an error message from sh about
> the command string argument being missing (except on a system were sh
> accepts -c with no command string as an extension, but I doubt any
> such system exists).
Testing:
$ bash -c '-i echo hello'
bash: - : invalid option
$ dash -c '-i echo hello'
dash: 0: Illegal option -
# android
$ sh -c "-i echo hello"
sh: sh: - : unknown option
So implementing system with arguments {"sh", "-c", "--", cmd, 0} shouldn't break
any existing use case that worked before.
>> Pinging you and Rich was my attempt at that, but if there's a dedicated
>> list...
>
> Note that I removed the libc-coord list from the Cc.
And removed the musl-libc maintainer for some reason.
Rob
Re: Posix issue 8 pending change to system().
2021-10-22 12:43:11 +0300, Oğuz:
[...]
> > So system() was broken when sh started accepting more than one
> > option argument.
> >
>
> I wouldn't say broken. This is rather an academic case, I don't see why
> anyone would name a utility that way (`-potato'/`+potato'), I don't know a
> single utility that is named that way either.
[...]
FWIW,
zsh has a "-" builtin command (to run a command with it's
argv[0] prefixed with a "-").
$ zsh -c '- ps -f'
zsh: bad option string: '- ps -f'
$ zsh -c -- '- ps -f'
UID PIDPPID C STIME TTY TIME CMD
chazelas 728758 93970 0 18:01 pts/800:00:00 /bin/zsh
chazelas 729347 728758 0 18:02 pts/800:00:00 -ps -f
(from 1990; less useful now that most shells support a -l /
--login to achieve the same effect as "- sh").
Note that it's not only about the command's name, one could want
to do:
system("++dir++/script.py blah");
> The standard should note this corner case and encourage developers to
> implement `system()' to behave as if `sh -c -- command' were called, and
> leave it at that.
[...]
Yes, it should make sure that system("any shell code") correctly
gets a shell to interpret that code.
--
Stephane
Re: Posix issue 8 pending change to system().
22 Ekim 2021 Cuma tarihinde Stephane Chazelas via austin-group-l at The
Open Group yazdı:
> 2021-10-22 01:11:43 -0500, Rob Landley via austin-group-l at The Open
> Group:
> [...]
> > > > Where system("-blah") fails because sh is insane legacy weirdness
> and it turns
> > > > out that -c does NOT take an argument. So:
> > > >
> > > > sh -c -i "echo hello"
> > > >
> > > > Works, which that means "sh -c -potato" tries to parse -potato as an
> option, and
> > > > fails.
> [...]
>
> Note that if it was even insaner legacy, it would be fine.
>
> In the Bourne shell, only the first argument was considered for
> options.
>
> In the Bourne shell originally, you'd have had to write:
>
> sh -fc 'echo hello'
>
> Or
>
> sh -cf 'echo hello'
>
> to interpret "echo hello" while the f option is enabled. sh -c
> -f 'echo hello' would interpret -f with "echo hello" in $0.
>
> So system() was broken when sh started accepting more than one
> option argument.
>
I wouldn't say broken. This is rather an academic case, I don't see why
anyone would name a utility that way (`-potato'/`+potato'), I don't know a
single utility that is named that way either.
The standard should note this corner case and encourage developers to
implement `system()' to behave as if `sh -c -- command' were called, and
leave it at that.
--
Oğuz
Re: Posix issue 8 pending change to system().
2021-10-22 01:11:43 -0500, Rob Landley via austin-group-l at The Open Group:
[...]
> > > Where system("-blah") fails because sh is insane legacy weirdness and it
> > > turns
> > > out that -c does NOT take an argument. So:
> > >
> > > sh -c -i "echo hello"
> > >
> > > Works, which that means "sh -c -potato" tries to parse -potato as an
> > > option, and
> > > fails.
[...]
Note that if it was even insaner legacy, it would be fine.
In the Bourne shell, only the first argument was considered for
options.
In the Bourne shell originally, you'd have had to write:
sh -fc 'echo hello'
Or
sh -cf 'echo hello'
to interpret "echo hello" while the f option is enabled. sh -c
-f 'echo hello' would interpret -f with "echo hello" in $0.
So system() was broken when sh started accepting more than one
option argument.
Similarly, the
#! /bin/sh -euf
scripts were broken in the same way for the same reason (modern
versions of sh still accept #! /bin/sh - as a way to mark the
end of options, but that can't be used in combination with
options in most shebang implementations) -- off topic here as
POSIX doesn't specify shebangs.
See also
https://unix.stackexchange.com/questions/351729/why-the-in-the-bin-sh-shebang
Note that it applies to sh -c +potato as well.
[...]
> > > it's unclear whether anyone's actually hit this in practice? and even if
> > > they
> > had, their portable workaround would be to prefix with "exec "?
[...]
An easier workaround is:
system(" -potato") or system("\n-potato").
It's the same workaround you need to use with eval:
eval " $code"
as IIRC, eval being a special builtin, is not required to accept
"--" as option delimiter (even though most implementations do;
dash and bosh being some exceptions).
Note that in some shells (and it's now required by POSIX), "exec
cmd" runs cmd from $PATH bypassing function and builtins (and
aliases for those shells that have builtin aliases and where
"exec" is not an alias for "exec ").
So prepending "exec" can affect functionality, though in most
cases using "exec" can be a good idea especially with shells
that don't optimise out the fork by themselves for the last
command.
--
Stephane
Re: Posix issue 8 pending change to system().
Rob Landley wrote, on 22 Oct 2021: > > On 10/21/21 6:38 PM, enh wrote: > > > it's unclear whether anyone's actually hit this in practice? and even if > > > they > > had, their portable workaround would be to prefix with "exec "? > > > > maybe try libc-co...@lists.openwall.com and see if there's any consensus > > that > > this is worth the potential trouble? i'm worried that someone is > > _deliberately_ using this to pass extra flags to the shell, which wouldn't > > have a workaround if we did make this change :-( It's not possible to pass "extra" flags when the "--" is not there, because system() only passes one argument after the "-c" to sh. So you can pass flags _instead_of_ a command string, but not _extra_ flags. Doing this will just get you an error message from sh about the command string argument being missing (except on a system were sh accepts -c with no command string as an extension, but I doubt any such system exists). > > Pinging you and Rich was my attempt at that, but if there's a dedicated > list... Note that I removed the libc-coord list from the Cc. -- Geoff Clare The Open Group, Apex Plaza, Forbury Road, Reading, RG1 1AX, England
Re: Posix issue 8 pending change to system().
On 10/21/21 6:38 PM, enh wrote:
> On Thu, Oct 21, 2021 at 10:56 AM Rob Landley wrote:
>
> > I was on the posix call today, and they have a bug:
> >
> > https://austingroupbugs.net/view.php?id=1440
> >
> > Where system("-blah") fails because sh is insane legacy weirdness and it
> > turns
> > out that -c does NOT take an argument. So:
> >
> > sh -c -i "echo hello"
> >
> > Works, which that means "sh -c -potato" tries to parse -potato as an
> > option, and
> > fails.
> >
> > The suggested fix is to change system() so that instead of {"sh", "-c",
> > command,
> > NULL} it does {"sh", "-c", "--", command, NULL} instead, but that can't go
> > into
> > an issue 7 tc because it's a behavior change which means it would go into
> > issue
> > 8. (Which they're working on now.)
> >
> > But they can't standardize a new feature without a "commitment to implement
> > it"
> > by at least one package maintainer, so I thought I'd give you guys a shout.
> > Looks trivial to implement in both codebases? Dash and android's shell both
> > work
> > with --...
> >
> > The minutes of the meeting aren't posted to the mailing list yet but
> > they're on
> > https://posix.rhansen.org/p/2021-10-21 for now. This part's at the bottom.
> >
> > Rob
> > it's unclear whether anyone's actually hit this in practice? and even if
> > they
> had, their portable workaround would be to prefix with "exec "?
>
> maybe try libc-co...@lists.openwall.com and see if there's any consensus that
> this is worth the potential trouble? i'm worried that someone is
> _deliberately_ using this to pass extra flags to the shell, which wouldn't
> have a workaround if we did make this change :-(
Pinging you and Rich was my attempt at that, but if there's a dedicated list...
Rob
