Skip to content

Commit

Permalink
Fix memory corruption when a compound variable is unset
Browse files Browse the repository at this point in the history
The following set of commands ends with a memory fault under
certain circumstances because ksh attempts to free memory
twice, causing memory corruption:

$ testarray=(1 2)
$ compound testarray
$ unset testarray
$ eval testarray=

The fix is to make sure 'np->nvfun' is a valid pointer before
attempting to free memory in 'put_tree'. This patch is from
OpenSUSE: https://build.opensuse.org/package/view_file/shells/ksh/ksh93-nvtree-free.dif?expand=1

src/cmd/ksh93/sh/nvtree.c:
- Do not try to free memory when 'np->nvfun' and 'val'
  are false.

src/cmd/ksh93/tests/comvar.sh:
- Add a regression test for the double free problem. The
  reproducer must be run from an executable script
  with 'ksh -c'.
  • Loading branch information
JohnoKing committed Jun 29, 2020
1 parent 5135cf6 commit 9c84744
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 0 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ Any uppercase BUG_* names are modernish shell bug IDs.
- Variables created with 'typeset -RF' no longer cause a memory fault
when accessed.

- Unsetting an array that was turned into a compound variable will no
longer cause silent memory corruption.

2020-06-26:

- Changing to a directory that has a name starting with a '.' will no
Expand Down
2 changes: 2 additions & 0 deletions src/cmd/ksh93/sh/nvtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -1142,6 +1142,8 @@ static void put_tree(register Namval_t *np, const char *val, int flags,Namfun_t
nv_putv(np, val, flags,fp);
if(val && nv_isattr(np,(NV_INTEGER|NV_BINARY)))
return;
if(!val && !np->nvfun)
return;
if(ap= nv_arrayptr(np))
nleft = array_elem(ap);
if(nleft==0)
Expand Down
24 changes: 24 additions & 0 deletions src/cmd/ksh93/tests/comvar.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ function err_exit
}
alias err_exit='err_exit $LINENO'

tmp=$(
d=${TMPDIR:-/tmp}/ksh93.comvar.$$.${RANDOM:-0}
mkdir -m700 -- "$d" && CDPATH= cd -P -- "$d" && pwd
) || {
err\_exit $LINENO 'mkdir failed'
exit 1
}
trap 'cd / && rm -rf "$tmp"' EXIT

#test for compound variables
Command=${0##*/}
integer Errors=0
Expand Down Expand Up @@ -691,4 +700,19 @@ xx=(foo=bar)
xx=()
[[ $xx == $'(\n)' ]] || err_exit 'xx=() not unsetting previous value'

# ======
# Unsetting an array turned into a compound variable shouldn't cause
# memory corruption. This test must be run as an executable script
# with 'ksh -c'.
compound_array=$tmp/compound_array.sh
cat >| "$compound_array" << 'EOF'
testarray=(1 2)
compound testarray
unset testarray
eval testarray=
EOF
chmod +x "$compound_array"
"$SHELL" -c "$compound_array" || err_exit 'unsetting an array turned into a compound variable fails'

# ======
exit $((Errors<125?Errors:125))

0 comments on commit 9c84744

Please sign in to comment.