Page MenuHomeFreeBSD

wordexp: Rework to make WRDE_NOCMD reliable.
AbandonedPublic

Authored by jilles on Sep 9 2015, 9:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 29 2024, 9:27 AM
Unknown Object (File)
Dec 24 2023, 10:28 AM
Unknown Object (File)
Dec 24 2023, 10:28 AM
Unknown Object (File)
Dec 20 2023, 12:49 AM
Unknown Object (File)
Oct 18 2023, 12:21 PM
Unknown Object (File)
Oct 14 2023, 7:29 AM
Unknown Object (File)
Sep 16 2023, 3:53 AM
Unknown Object (File)
Aug 1 2023, 1:51 PM
Subscribers

Details

Reviewers
None
Group Reviewers
manpages
Summary

It appears that there is some wordexp() use that may depend on security
of WRDE_NOCMD. The current wordexp() allows arbitrary command execution
even if WRDE_NOCMD is set, since shell syntax is too complicated to detect
command substitution and unquoted operators reliably without implementing
much of sh's parser. This diff fixes this by adding some functionality to sh
(as opposed to implementing a full shell parser in libc).

The new functionality is an undocumented builtin utility freebsd_wordexp
that invokes the parser and expansion code. The old undocumented builtin
utility wordexp may be removed at some point.

The basic concept is:
execl("/bin/sh", "sh", "-c", "freebsd_wordexp ${1:+\"$1\"} -f "$2",
"", flags & WRDE_NOCMD ? "-p" : "", <pipe with words>);

Apart from implementing wordexp(), freebsd_wordexp is also useful to
fuzz more of sh than can be reached via sh -n. I fixed two bugs in the
expansion code via fuzzing (already committed as r287081 and r287148).

I may use this freebsd_ prefix more often for non-standard functionality.

While changing sh's support anyway, also read input from a pipe instead of
arguments to avoid {ARG_MAX} limits and improve privacy, and output count
and length using 16 instead of 8 digits.

The WRDE_BADCHAR error is still implemented in libc. POSIX requires us to
fail strings containing unquoted braces with code WRDE_BADCHAR. Since this
is normally not a syntax error in sh, there is still a need for checking
code in libc, we_check().

The new we_check() is an optimistic check that all the characters

<newline> | & ; < > ( ) { }

are quoted. To avoid duplicating too much sh logic, such characters are
permitted when quoting characters are seen, even if the quoting characters
may themselves be quoted. This code reports all WRDE_BADCHAR errors; bad
characters that get past it and are a syntax error in sh return WRDE_SYNTAX.

Test Plan

tools/regression/lib/libc/gen/test-wordexp.c

With an older version:
afl-fuzz -i testcases1 -o findings1 ./sh -c 'exec 7<&0; freebsd_wordexp -p -f 7'
with a single file containing $IFS as seed.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 495
Build 495: arc lint + arc unit

Event Timeline

jilles retitled this revision from to wordexp: Rewrite to make WRDE_NOCMD reliable..
jilles updated this object.
jilles edited the test plan for this revision. (Show Details)
jilles retitled this revision from wordexp: Rewrite to make WRDE_NOCMD reliable. to wordexp: Rework to make WRDE_NOCMD reliable..Sep 10 2015, 8:44 PM
jilles updated this object.
jilles edited edge metadata.

Fix bug in arglist handling.

wblock added inline comments.
lib/libc/gen/wordexp.3
28

Remember to update this.

111

Why say "undocumented" here?

200
Pathname generation may create output that is exponentially larger than the input size.
lib/libc/gen/wordexp.3
28

I leave the date unchanged in patches to avoid annoying conflicts :)

111

Because that's what the old man page said ;)

The point is that wordexp() will not work if you replace /bin/sh with something else. In a FreeBSD context that is clearly a bad idea, but this code is sometimes used outside a FreeBSD context.

200

That sounds better and will be in the next iteration.

Committed to head as SVN r288430 with the change to the man page. Thanks Warren Block for the review.