Skip to content

Commit 3654ee7

Browse files
committed
Fix typeset -l/-u crash on special vars (rhbz#1083713)
When using typeset -l or -u on a variable that cannot be changed when the shell is in restricted mode, ksh crashed. This fixed is inspired by this Red Hat fix, which is incomplete: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-tpstl.patch The crash was caused by the nv_shell() function. It walks though a discipline function tree to get the pointer to the interpreter associated with it. Evidently, the problem is that some pointer in that walk is not set correctly for all special variables. Thing is, ksh only has one shell language interpreter, and only one global data structure (called 'sh') to keep its main state[*]. Yet, the code is full of 'shp' pointers to that structure. Most (not all) functions pass that pointer around to each other, accessing that struct indirectly, ostensibly to account for the non-existent possibility that there might be more than one interpreter state. The "why" of that is an interesting cause for speculation that I may get to sometime. For now, it is enough to know that, in the code as it is, it matters not one iota what pointer to the shell interpreter state is used; they all point to the same thing (unless it's broken, as in this bug). So, rather than fixing nv_shell() and/or associated pointer assignments, this commit simply removes it, and replaces it with calls to sh_getinterp(), which always returns a pointer to sh (see init.c, where that function is defined as literally 'return &sh'). [*] Defined in shell.h, with the _SH_PRIVATE part in defs.h src/cmd/ksh93/include/defs.h, src/cmd/ksh93/sh/name.c: - Remove nv_shell(). src/cmd/ksh93/sh/init.c: - In all the discipline functions for special variables, initialise shp using sh_getinterp() instead of nv_shell(). src/cmd/ksh93/tests/variables.sh: - Add regression test for typeset -l/-u on all special variables.
1 parent 843b546 commit 3654ee7

File tree

5 files changed

+62
-35
lines changed

5 files changed

+62
-35
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ Any uppercase BUG_* names are modernish shell bug IDs.
88
- Fixed a crash that could occur when running a pipeline containing
99
backtick-style command substitutions with job control enabled.
1010

11+
- Fixed a crash that occurred when using 'typeset -u' or 'typeset -l' on a
12+
special variable such as PATH, ENV or SHELL.
13+
1114
2020-09-21:
1215

1316
- A bug was fixed that caused command substitutions embedded in here-documents

src/cmd/ksh93/include/defs.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,6 @@ struct shared
359359
#define SH_FUNEVAL 0x10000 /* for sh_eval for function load */
360360

361361
extern struct shared *shgd;
362-
extern Shell_t *nv_shell(Namval_t*);
363362
extern void sh_applyopts(Shell_t*,Shopt_t);
364363
extern char **sh_argbuild(Shell_t*,int*,const struct comnod*,int);
365364
extern struct dolnod *sh_argfree(Shell_t *, struct dolnod*,int);

src/cmd/ksh93/sh/init.c

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ static void put_ed(register Namval_t* np,const char *val,int flags,Namfun_t *fp)
227227
{
228228
register const char *cp, *name=nv_name(np);
229229
register int newopt=0;
230-
Shell_t *shp = nv_shell(np);
230+
Shell_t *shp = sh_getinterp();
231231
if(*name=='E' && nv_getval(sh_scoped(shp,VISINOD)))
232232
goto done;
233233
if(!(cp=val) && (*name=='E' || !(cp=nv_getval(sh_scoped(shp,EDITNOD)))))
@@ -254,7 +254,7 @@ static void put_ed(register Namval_t* np,const char *val,int flags,Namfun_t *fp)
254254
/* Trap for HISTFILE and HISTSIZE variables */
255255
static void put_history(register Namval_t* np,const char *val,int flags,Namfun_t *fp)
256256
{
257-
Shell_t *shp = nv_shell(np);
257+
Shell_t *shp = sh_getinterp();
258258
void *histopen = shp->gd->hist_ptr;
259259
char *cp;
260260
if(val && histopen)
@@ -278,7 +278,7 @@ static void put_history(register Namval_t* np,const char *val,int flags,Namfun_t
278278
/* Trap for OPTINDEX */
279279
static void put_optindex(Namval_t* np,const char *val,int flags,Namfun_t *fp)
280280
{
281-
Shell_t *shp = nv_shell(np);
281+
Shell_t *shp = sh_getinterp();
282282
shp->st.opterror = shp->st.optchar = 0;
283283
nv_putv(np, val, flags, fp);
284284
if(!val)
@@ -303,7 +303,7 @@ static Namfun_t *clone_optindex(Namval_t* np, Namval_t *mp, int flags, Namfun_t
303303
/* Trap for restricted variables FPATH, PATH, SHELL, ENV */
304304
static void put_restricted(register Namval_t* np,const char *val,int flags,Namfun_t *fp)
305305
{
306-
Shell_t *shp = nv_shell(np);
306+
Shell_t *shp = sh_getinterp();
307307
int path_scoped = 0, fpath_scoped=0;
308308
Pathcomp_t *pp;
309309
char *name = nv_name(np);
@@ -345,7 +345,7 @@ static void put_restricted(register Namval_t* np,const char *val,int flags,Namfu
345345
static void put_cdpath(register Namval_t* np,const char *val,int flags,Namfun_t *fp)
346346
{
347347
Pathcomp_t *pp;
348-
Shell_t *shp = nv_shell(np);
348+
Shell_t *shp = sh_getinterp();
349349
nv_putv(np, val, flags, fp);
350350
if(!shp->cdpathlist)
351351
return;
@@ -369,7 +369,7 @@ static void put_cdpath(register Namval_t* np,const char *val,int flags,Namfun_t
369369
/* Trap for LC_ALL, LC_CTYPE, LC_MESSAGES, LC_COLLATE and LANG */
370370
static void put_lang(Namval_t* np,const char *val,int flags,Namfun_t *fp)
371371
{
372-
Shell_t *shp = nv_shell(np);
372+
Shell_t *shp = sh_getinterp();
373373
int type;
374374
char *name = nv_name(np);
375375
if(name==(LCALLNOD)->nvname)
@@ -494,7 +494,7 @@ static char* get_ifs(register Namval_t* np, Namfun_t *fp)
494494
register struct ifs *ip = (struct ifs*)fp;
495495
register char *cp, *value;
496496
register int c,n;
497-
register Shell_t *shp = nv_shell(np);
497+
register Shell_t *shp = sh_getinterp();
498498
value = nv_getv(np,fp);
499499
if(np!=ip->ifsnp)
500500
{
@@ -573,7 +573,7 @@ static void put_seconds(register Namval_t* np,const char *val,int flags,Namfun_t
573573

574574
static char* get_seconds(register Namval_t* np, Namfun_t *fp)
575575
{
576-
Shell_t *shp = nv_shell(np);
576+
Shell_t *shp = sh_getinterp();
577577
register int places = nv_size(np);
578578
struct tms tp;
579579
double d, offset = (np->nvalue.dp?*np->nvalue.dp:0);
@@ -657,7 +657,7 @@ static Sfdouble_t nget_lineno(Namval_t* np, Namfun_t *fp)
657657
static void put_lineno(Namval_t* np,const char *val,int flags,Namfun_t *fp)
658658
{
659659
register long n;
660-
Shell_t *shp = nv_shell(np);
660+
Shell_t *shp = sh_getinterp();
661661
if(!val)
662662
{
663663
fp = nv_stack(np, NIL(Namfun_t*));
@@ -681,7 +681,7 @@ static char* get_lineno(register Namval_t* np, Namfun_t *fp)
681681

682682
static char* get_lastarg(Namval_t* np, Namfun_t *fp)
683683
{
684-
Shell_t *shp = nv_shell(np);
684+
Shell_t *shp = sh_getinterp();
685685
char *cp;
686686
int pid;
687687
if(sh_isstate(SH_INIT) && (cp=shp->lastarg) && *cp=='*' && (pid=strtol(cp+1,&cp,10)) && *cp=='*')
@@ -691,7 +691,7 @@ static char* get_lastarg(Namval_t* np, Namfun_t *fp)
691691

692692
static void put_lastarg(Namval_t* np,const char *val,int flags,Namfun_t *fp)
693693
{
694-
Shell_t *shp = nv_shell(np);
694+
Shell_t *shp = sh_getinterp();
695695
if(flags&NV_INTEGER)
696696
{
697697
sfprintf(shp->strbuf,"%.*g",12,*((double*)val));
@@ -934,7 +934,7 @@ static void math_init(Shell_t *shp)
934934

935935
static Namval_t *create_math(Namval_t *np,const char *name,int flag,Namfun_t *fp)
936936
{
937-
Shell_t *shp = nv_shell(np);
937+
Shell_t *shp = sh_getinterp();
938938
if(!name)
939939
return(SH_MATHNOD);
940940
if(name[0]!='a' || name[1]!='r' || name[2]!='g' || name[4] || !isdigit(name[3]) || (name[3]=='0' || (name[3]-'0')>MAX_MATH_ARGS))
@@ -945,7 +945,7 @@ static Namval_t *create_math(Namval_t *np,const char *name,int flag,Namfun_t *fp
945945

946946
static char* get_math(register Namval_t* np, Namfun_t *fp)
947947
{
948-
Shell_t *shp = nv_shell(np);
948+
Shell_t *shp = sh_getinterp();
949949
Namval_t *mp,fake;
950950
char *val;
951951
int first=0;
@@ -966,7 +966,7 @@ static char* get_math(register Namval_t* np, Namfun_t *fp)
966966

967967
static char *setdisc_any(Namval_t *np, const char *event, Namval_t *action, Namfun_t *fp)
968968
{
969-
Shell_t *shp=nv_shell(np);
969+
Shell_t *shp=sh_getinterp();
970970
Namval_t *mp,fake;
971971
char *name;
972972
int getname=0, off=staktell();

src/cmd/ksh93/sh/name.c

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3624,17 +3624,6 @@ int nv_setsize(register Namval_t *np, int size)
36243624
return(oldsize);
36253625
}
36263626

3627-
Shell_t *nv_shell(Namval_t *np)
3628-
{
3629-
Namfun_t *fp;
3630-
for(fp=np->nvfun;fp;fp=fp->next)
3631-
{
3632-
if(!fp->disc)
3633-
return((Shell_t*)fp->last);
3634-
}
3635-
return(0);
3636-
}
3637-
36383627
#undef nv_unset
36393628

36403629
void nv_unset(register Namval_t *np)

src/cmd/ksh93/tests/variables.sh

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,7 @@ actual=$(env SHLVL="2#11+x[\$(env echo Exploited vuln CVE-2019-14868 >&2)0]" "$S
868868
[[ $actual == $expect ]] || err_exit "expression allowed on env var import (expected '$expect', got '$actual')"
869869

870870
# ======
871-
# Check unset and cleanup/restore behavior of special variables.
871+
# Check unset, attribute and cleanup/restore behavior of special variables.
872872

873873
# Keep the list in sync (minus ".sh") with shtab_variables[] in src/cmd/ksh93/data/variables.c
874874
# Note: as long as changing $PATH forks a virtual subshell, "PATH" should also be excluded below.
@@ -954,8 +954,8 @@ $SHELL -c '
954954
done
955955
exit $((errors + 1)) # a possible erroneous asynchronous fork would cause exit status 0
956956
' unset_test "$@"
957-
e=$?
958-
((e == 1)) || err_exit "Failure in unsetting one or more special variables (exit status $e)"
957+
(((e = $?) == 1)) || err_exit "Failure in unsetting one or more special variables" \
958+
"(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))"
959959

960960
# ... unset in virtual subshell inside of nested function
961961
$SHELL -c '
@@ -984,8 +984,8 @@ $SHELL -c '
984984
fun1 "$@"
985985
exit $((errors + 1)) # a possible erroneous asynchronous fork would cause exit status 0
986986
' unset_subsh_fun_test "$@"
987-
e=$?
988-
((e == 1)) || err_exit "Unset of special variable(s) in a virtual subshell within a nested function fails (exit status $e)"
987+
(((e = $?) == 1)) || err_exit "Unset of special variable(s) in a virtual subshell within a nested function fails" \
988+
"(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))"
989989

990990
# ... readonly in subshell
991991
$SHELL -c '
@@ -1008,8 +1008,8 @@ $SHELL -c '
10081008
done
10091009
exit $((errors + 1)) # a possible erroneous asynchronous fork would cause exit status 0
10101010
' readonly_test "$@"
1011-
e=$?
1012-
((e == 1)) || err_exit "Failure in making one or more special variables readonly in a subshell (exit status $e)"
1011+
(((e = $?) == 1)) || err_exit "Failure in making one or more special variables readonly in a subshell" \
1012+
"(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))"
10131013

10141014
# ... subshell leak test
10151015
$SHELL -c '
@@ -1025,8 +1025,44 @@ $SHELL -c '
10251025
done
10261026
exit $((errors + 1))
10271027
' subshell_leak_test "$@"
1028-
e=$?
1029-
((e == 1)) || err_exit "One or more special variables leak out of a subshell (exit status $e)"
1028+
(((e = $?) == 1)) || err_exit "One or more special variables leak out of a subshell" \
1029+
"(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))"
1030+
1031+
# ... upper/lowercase test
1032+
$SHELL -c '
1033+
typeset -u upper
1034+
typeset -l lower
1035+
errors=0
1036+
PS1=/dev/null/test_my_case_too
1037+
PS2=$PS1 PS3=$PS1 PS4=$PS1 OPTARG=$PS1 IFS=$PS1 FPATH=$PS1 FIGNORE=$PS1 CSWIDTH=$PS1
1038+
for var
1039+
do case $var in
1040+
RANDOM | HISTCMD | _ | SECONDS | LINENO | JOBMAX | .sh.stats)
1041+
# these are expected to fail below as their values change; just test against crashing
1042+
typeset -u "$var"
1043+
typeset -l "$var"
1044+
continue ;;
1045+
esac
1046+
nameref val=$var
1047+
upper=$val
1048+
lower=$val
1049+
typeset -u "$var"
1050+
if [[ $val != "$upper" ]]
1051+
then echo " $0: typeset -u does not work on special variable $var" \
1052+
"(expected $(printf %q "$upper"), got $(printf %q "$val"))" >&2
1053+
let errors++
1054+
fi
1055+
typeset -l "$var"
1056+
if [[ $val != "$lower" ]]
1057+
then echo " $0: typeset -l does not work on special variable $var" \
1058+
"(expected $(printf %q "$lower"), got $(printf %q "$val"))" >&2
1059+
let errors++
1060+
fi
1061+
done
1062+
exit $((errors + 1))
1063+
' changecase_test "$@" PATH # do include PATH here as well
1064+
(((e = $?) == 1)) || err_exit "typeset -l/-u doesn't work on special variables" \
1065+
"(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))"
10301066

10311067
# ======
10321068
# ${.sh.pid} should be the forked subshell's PID

0 commit comments

Comments
 (0)