1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00

Fix arbitrary command execution vuln in array subscripts in arith

This commit fixes an arbitrary command execution vulnerability in
array subscripts used within the arithmetic subsystem.

One of the possible reproducers is:
	var='1$(echo INJECTION >&2)' ksh -c \
		'typeset -A a; ((a[$var]++)); typeset -p a'

Output before this commit:
	INJECTION
	typeset -A a=([1]=1)
The 'echo' command has been surreptitiously executed from an
external environment variable.

Output after this commit:
	typeset -A a=(['1$(echo INJECTION >&2)']=1)
The value is correctly used as an array subscript and nothing in it
is parsed or executed. This is as it should be, as ksh93 supports
arbitrary subscripts for associative arrays.

If we think about it logically, the C-style arithmetic subsystem
simply has no business messing around with shell expansions or
quoting at all, because those don't belong to it. Shell expansions
and quotes are properly resolved by the main shell language before
the arithmetic subsystem is even invoked. It is particularly
important to maintain that separation because the shell expansion
mechanism also executes command substitutions.

Yet, the arithmetic subsystem subjected array subscripts that
contain `$` (and only array subscripts -- how oddly specific) to
an additional level of expansion and quote resolution. For some
unfathomable reason, there are two lines of code doing specifically
this. The vulnerability is fixed by simply removing those.

Incredibly, variants of this vulnerability are shared by bash, mksh
and zsh. Instead of fixing it, it got listed in Bash Pitfalls!
http://mywiki.wooledge.org/BashPitfalls#y.3D.24.28.28_array.5B.24x.5D_.29.29

src/cmd/ksh93/sh/arith.c:
- scope(): Remove these two lines that implement the vulnerability.
			if(strchr(sub,'$'))
				sub = sh_mactrim(shp,sub,0);
- scope(), arith(): Remove the NV_SUBQUOTE flag from two
  nv_endsubscript() calls. That flag causes the array subscript to
  retain the current level of shell quoting. The shell quotes
  everything as in "double quotes" before invoking the arithmetic
  subsystem, and the bad sh_mactrim() call removed one level of
  quoting. Since we're no longer doing that, this flag should no
  longer be passed, or subscripts may get extra backslash escapes.

src/cmd/ksh93/include/name.h,
src/cmd/ksh93/sh/array.c:
- nv_endsubscript(): The NV_SUBQUOTE flag was only passed from
  arith.c. Since it is now unused, remove it.

src/cmd/ksh93/tests/arith.sh:
- Tweak some tests: fix typos, report wrong values.
- Add 21 tests. Most are based on reproducers contributed by
  @stephane-chazelas and @hyenias. They verify that this
  vulnerability is gone and that no quoting bugs were introduced.

Resolves: https://github.com/ksh93/ksh/issues/152
This commit is contained in:
Martijn Dekker 2021-03-04 13:37:13 +00:00
parent a61430f1b5
commit b48e5b3365
7 changed files with 91 additions and 32 deletions

5
NEWS
View file

@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh
Any uppercase BUG_* names are modernish shell bug IDs.
2021-03-04:
- Fixed an arbitrary command execution vulnerability that occurred when
parsing the subscripts of arrays within arithmetic commands and expansion.
2021-03-01:
- Fixed the retention of size attributes when 'readonly' or 'typeset -r'