[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260630212908.GY3520958@port70.net>
Date: Tue, 30 Jun 2026 23:29:08 +0200
From: Szabolcs Nagy <nsz@...t70.net>
To: Rich Felker <dalias@...c.org>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH 0/4] wordexp fixes
* Rich Felker <dalias@...c.org> [2026-06-30 15:53:33 -0400]:
> On Tue, Jun 30, 2026 at 04:31:33PM +0200, Szabolcs Nagy wrote:
> > @@ -108,9 +107,13 @@ static int do_wordexp(const char *s, wordexp_t *we, int flags)
> > if (!pid) {
> > if (p[1] == 1) fcntl(1, F_SETFD, 0);
> > else dup2(p[1], 1);
> > + if (!(flags & WRDE_SHOWERR)) {
> > + int fd = open("/dev/null", O_WRONLY|O_CLOEXEC);
> > + if (fd < 0 || dup2(fd, 2) < 0) close(2);
> > + }
> > execl("/bin/sh", "sh", "-c",
> > - "eval \"printf %s\\\\\\\\0 x $1 $2\"",
> > - "sh", s, redir, (char *)0);
> > + "eval \"printf %s\\\\\\\\0 x $1\"",
> > + "sh", s, (char *)0);
> > _exit(1);
> > }
> > close(p[1]);
> > --
> > 2.52.0
>
> I don't think silently proceeding with close(2) is reasonable here. It
> will potentially cause any descendant to corrupt other output by
> mixing in things intended for stderr.
ah i didnt think of that.
>
> If we need to open /dev/null ourselves, it should probably be done in
> the parent with only the dup2 in the child. This way the error can be
> handled before there's even a child.
ok.
> >
> > wordexp("$*", p, 0)
> >
> > was "$*","2>/dev/stderr", now empty list.
...
> > @@ -112,7 +112,7 @@ static int do_wordexp(const char *s, wordexp_t *we, int flags)
> > if (fd < 0 || dup2(fd, 2) < 0) close(2);
> > }
> > execl("/bin/sh", "sh", "-c",
> > - "eval \"printf %s\\\\\\\\0 x $1\"",
> > + "eval \"set --;printf %s\\\\\\\\0 x $1\"",
> > "sh", s, (char *)0);
> > _exit(1);
> > }
>
> I think this is right. It's in the eval so as not to affect the $1
> used in the eval expression, right?
yes
> > wordexp("$(echo x $U) y", p, WRDE_UNDEF|WRDE_SHOWERR)
> >
> > was "x","y", now "y" and prints an error about U. the echo is not
> > executed, but wordexp succeeds. undef failure is not propagated from
> > a command substitution (but it inherits -u).
>
> I think this is contrary to the behavior as specified. The $U here is
> not an expansion by the wordexp, but an expansion in the command
> inside $(), which is not specified to be subject to set -u. However it
> is somewhat ambiguous.
>
> But as you've noted, if the WRDE_UNDEF should affect things inside
> $(), it's still wrong because wordexp isn't failing.
>
> I think it's clear that the behavior should be either success or an
> error from wordexp, not just a silent error from the $().
one option is to only handle WRDE_NOCMD|WRDE_UNDEF
(otherwise fail with syntax?)
or we can try sh -u with monitored fd 2 and assume
any write to fd 2 is either syntax or undef error.
this would catch subshell errors, but also user code
printing to stderr...
we can rerun with sh -n to detect undef vs syntax errors
> > on failure rerun with sh -n to distinguish unset from syntax errors
> > without side-effects. for the syntax check the string is passed
> > directly as a command which can be parsed differently:
>
> Nice, I think this approach solves the problem of double side effects.
>
> > wordexp("{ $U", p, 0)
> > wordexp("{ $U", p, WRDE_UNDEF)
> >
> > former is "{" but latter fails with WRDE_SYNTAX, not WRDE_UNDEF.
> > (note: using eval "set -n --;..." does not work on mksh, oksh.)
> > we == NULL marks the rerun (ub for user code).
> >
> > wordexp("$(echo>>x) $U $(echo>>x)", p, WRDE_UNDEF)
> > wordexp("$(echo>>x) $U $(echo>>x", p, WRDE_UNDEF)
> >
> > former returns WRDE_BADVAL and writes one \n to x, latter returns
> > WRDE_SYNTAX and has no side-effect.
> > Subject: [PATCH 4/4] wordexp: reimplement WRDE_NOCMD
...
>
> This needs to be done, but it's a lot to review. I think I'll start
> with some examples the current code doesn't handle right and work
> through how the proposed change fixes them, but it might take a while.
yeah parsing of shell syntax is complicated.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
