Skip to content

Commit

Permalink
Fix crash on redefining array in subshell (re: 92f65cb)
Browse files Browse the repository at this point in the history
The referenced commit left one test unexecuted because it crashes.

Minimal reproducer:

  typeset -a arr=((a b c) 1)
  got=$( typeset -a arr=( ( ((a b c)1))) )

The crash occurs when the array is redefined in a subshell.

Here are abridged ASan stack traces for the crash, for the use
after free, and for when it was freed:

=================================================================
==73147==ERROR: AddressSanitizer: heap-use-after-free [snippage]
READ of size 8 at 0x000107403eb0 thread T0
    #0 0x104fded40 in nv_search nvdisc.c:1007
    #1 0x104fbeb1c in nv_create name.c:860
    #2 0x104fb8b9c in nv_open name.c:1440
    #3 0x104fb1edc in nv_setlist name.c:309
    #4 0x104fb4a30 in nv_setlist name.c:475
    #5 0x105055b58 in sh_exec xec.c:1079
    #6 0x105045cd4 in sh_subshell subshell.c:654
    #7 0x104f92c1c in comsubst macro.c:2266
[snippage]

0x000107403eb0 is located 0 bytes inside of 80-byte region [snippage]
freed by thread T0 here:
    #0 0x105c5ade4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4)
    #1 0x105261da0 in dtclose dtclose.c:52
    #2 0x104f178cc in array_putval array.c:671
    #3 0x104fd7f4c in nv_putv nvdisc.c:144
    #4 0x104fbc5f0 in _nv_unset name.c:2435
    #5 0x104fb3250 in nv_setlist name.c:364
    #6 0x105055b58 in sh_exec xec.c:1079
    #7 0x105045cd4 in sh_subshell subshell.c:654
    #8 0x104f92c1c in comsubst macro.c:2266
[snippage]

So the crash is caused because array_putval (array.c:671) calls
dtclose, freeing ap->table, which is then reused after a recursive
nv_setlist call via nv_open() -> nv_create() -> nv_search().
This only happens whwn we're in a virtual subshell.

src/cmd/ksh93/sh/array.c:
- array_putval(): When redefining an array in a virtual subshell,
  do not free the old ap->table; it will be needed by the parent
  shell environment.
  • Loading branch information
McDutchie committed Mar 11, 2024
1 parent 50c1b79 commit 244b96b
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 9 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
- Fixed a longstanding bug where the default terminal width for typeset -L, -R,
or -Z, if not given, was miscalculated for multibyte or control characters.

- Fixed a crash on redefining an array inherited from the parent environment
in a subshell.

2024-03-07:

- Fixed a bug that caused some systems to corrupt the display of multibyte
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ static void array_putval(Namval_t *np, const char *string, int flags, Namfun_t *
{
if(is_associative(ap))
(*ap->fun)(np, NULL, NV_AFREE);
else if(ap->table)
else if(ap->table && (!sh.subshell || sh.subshare))
{
dtclose(ap->table);
ap->table = 0;
Expand Down
12 changes: 4 additions & 8 deletions src/cmd/ksh93/tests/basic.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# #
# This software is part of the ast package #
# Copyright (c) 1982-2012 AT&T Intellectual Property #
# Copyright (c) 2020-2023 Contributors to ksh 93u+m #
# Copyright (c) 2020-2024 Contributors to ksh 93u+m #
# and is licensed under the #
# Eclipse Public License, Version 2.0 #
# #
Expand Down Expand Up @@ -1003,7 +1003,6 @@ done
# ======
# Nested compound assignment misparsed in $(...) or ${ ...; } command substitution
# https://github.com/ksh93/ksh/issues/269
# TODO: one of the tests below crashes when actually executed; test lexing only by using noexec.
for testcode in \
': $( typeset -a arr=((a b c) 1) )' \
': ${ typeset -a arr=((a b c) 1); }' \
Expand All @@ -1018,14 +1017,11 @@ for testcode in \
'typeset -Ca arr=((a=ah b=beh c=si))' \
': $( typeset -Ca arr=((a=ah b=beh c=si)) )' \
'r=${ typeset -Ca arr=((a=ah b=beh c=si)); }' \
'set --noexec; : $( typeset -a arr=((a $(( $( typeset -a barr=((a $(( 1 << 2 )) c) 1); echo 1 ) << $( typeset -a bazz=((a $(( 1 << 2 )) c) 1); echo 2 ) )) c) 1) )' \
': $( typeset -a arr=((a $(( $( typeset -a barr=((a $(( 1 << 2 )) c) 1); echo 1 ) << $( typeset -a bazz=((a $(( 1 << 2 )) c) 1); echo 2 ) )) c) 1) )' \
'r=$(typeset -C arr=( (a=ah b=beh c=si) 1 (e f g)));'
do
# fork comsub with 'ulimit' on old ksh to avoid a fixed lexer bug crashing the entire test script
got=$(let ".sh.version >= 20211209" || ulimit -c 0
eval "set +x; $testcode" 2>&1) \
|| err_exit "comsub/arithexp lexing test $(printf %q "$testcode"):" \
"got status $? and $(printf %q "$got")"
got=$(export testcode; "$SHELL" -c 'v=$(eval "$testcode" 2>&1); e=$?; print -r -- "$v"; exit $e' 2>&1) \
|| { e=$?; err_exit "comsub/arithexp lexing test $(printf %q "$testcode"): got status $e and $(printf %q "$got")"; }
done
unset testcode
Expand Down

0 comments on commit 244b96b

Please sign in to comment.