oss-sec mailing list archives

Re: shell wildcard expansion (un)safety


From: Steffen Nurpmeso <steffen () sdaoden eu>
Date: Thu, 07 Nov 2024 22:04:20 +0100

Solar Designer wrote in
 <20241107041658.GA10363 () openwall com>:
 |On Thu, Nov 07, 2024 at 01:08:19AM +0100, Steffen Nurpmeso wrote:
 |> David A. Wheeler wrote in
 |>  <F60236E0-F65A-4441-9E62-64EE55016B2C () dwheeler com>:
 |>|> On Nov 5, 2024, at 11:12 PM, Solar Designer <solar () openwall com> wrote:
 |>|
 |>|> ... over the years we gained things like ...
 |>|> 
 |>|> find . -mindepth 1 -maxdepth 1 -type f -print0 | xargs -0 grep text --
 |>|
 |>|The "-print0" and "-0" options have been widely implemented, but
 |>|POSIX 2024 finally formally adds them. So I urge using them where they
 |>|make sense, as they counter embedded linefeed characters in filenames.
 |> 
 |> To add that the POSIX core developers mention (APPLICATION USAGE):
 |> 
 |>   It should be noted that using find with -print0 to pipe input to
 |>   xargs -r0 is less safe than using find with -exec because if
 |>   find -print0 is terminated after it has written a partial
 |>   pathname, the partial pathname may be processed as if it was
 |>   a complete pathname.
 |
 |Shouldn't that behavior be treated as an xargs implementation bug or at
 |least shortcoming, and fixed as such?  I hope POSIX doesn't require it?

Now, i am not a POSIX core developer.  POSIX.1-2024 was developed
for over a decade (even almost one and a half) with many hundreds
of issues fixed through discussions in regular meetings.
A first thought is that the now really included (four decades too
late!) sh(1)ell's "pipefail" option was agreed upon long after the
text above appeared for the -print0/-r0 addition.  If that is true
the above text is anyway a correct statement less the partial
pathname because the undesired "termination" will not be reflected
in the exit status of the pipe.

 |In other words, if the input stream to "xargs -0" doesn't end in a NUL,
 |xargs must not process the last maybe-partial string.  I've just checked

Other than that i would agree.

 |GNU findutils xargs (not the latest version, though) and it does have
 |this problem - something we'd want to fix?

From a glance "git show master:findutils/xargs.c::process0_stdin()"
of busybox also does

                int c = getchar();
                if (c == EOF) {
                        if (p == s)
                                goto ret;
                        c = '\0';
                }
                *p++ = c;
                if (c == '\0') {   /* NUL or EOF detected */

 ...

So then the above paragraph even reflects code reality.

--steffen
|
|Der Kragenbaer,                The moon bear,
|der holt sich munter           he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)
|
|And in Fall, feel "The Dropbear Bard"s ball(s).
|
|The banded bear
|without a care,
|Banged on himself fore'er and e'er
|
|Farewell, dear collar bear


Current thread: