Skip to content

Commit

Permalink
Fix crash on arith 'for' with one ';' (re: 1a30a27)
Browse files Browse the repository at this point in the history
We weren't done yet after fixing the infinite loop. The fix
unmasked another crashing bug that has been in ksh since forever:

  $ ksh -c 'for((;));do :;done'
  Memory fault
  $ ksh -c 'for((1;));do :;done'
  Memory fault

Any arithmetic 'for' with just one ';' in it, instead of the
expected two, will crash the shell. This bug was masked by the
infinite loop introduced in f8f2c4b and unmasked by 1a30a27, but
it has been in ksh93 since forever.

Analysis: arithfor() counts the semicolins in n and expects n==2.
But its test (fcpeek(-1)!=',') does not work if there is one
semicolon instead of the expected two; the sh_lexskip() call does
not advance the input pointer in that case so the test remains true
and 'n' ends up as 2 even if there is only one ';'.

What happens in that case, now that we no longer throw a syntax
error in sh_lex() lex.c:347/375, is that the loop breaks (lines
377, 1247) and the token is set to zero (lines 1458, 1514). So that
is what we need to test for in arithfor().

src/cmd/ksh93/sh/parse.c: arithfor():
- Replace the broken check; break the loop if lexp->token is zero
  after the sh_lexskip call, so that we can throw the expected
  syntax error if n < 2, after restoring state (on lines 784-788).
  • Loading branch information
McDutchie committed Aug 28, 2024
1 parent 1a30a27 commit 5aed2aa
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 10 deletions.
5 changes: 5 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ This documents significant changes in the dev branch of ksh 93u+m.
For full details, see the git log at: https://github.com/ksh93/ksh
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.

2024-08-28:

- Fixed a crash that occurred for an arithmetic 'for' that has two instead of
three ';'-separated expressions. The expected syntax error is now given.

2024-08-26:

- Fixed an infinite loop that occurred when a syntax error was thrown
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.1.0-alpha" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2024-08-26" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2024-08-28" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2024 Contributors to ksh " SH_RELEASE_FORK

/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ static Shnode_t *arithfor(Lex_t *lexp,Shnode_t *tf)
/* copy up to ; onto the stack */
sh_lexskip(lexp,';',1,ST_NESTED);
offset = stktell(sh.stk)-1;
if((c=fcpeek(-1))!=';')
if(!lexp->token)
break;
/* remove trailing white space */
while(offset>ARGVAL && ((c= *stkptr(sh.stk,offset-1)),isspace(c)))
Expand Down
13 changes: 5 additions & 8 deletions src/cmd/ksh93/tests/loop.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,11 @@ exp=a1xb1xc1x
# ======
# arithmetic for

got=$(eval 'for((i=0,i<10,i++)); do :; done' 2>&1)
exp=': syntax error at line 1: `))'\'' unexpected'
[[ $got == *"$exp" ]] || err_exit "arithmetic for syntax error" \
"(expected match of *$(printf %q "$exp"), got $(printf %q "$got"))"
got=$(eval 'for(())' 2>&1)
exp=': syntax error at line 1: `))'\'' unexpected'
[[ $got == *"$exp" ]] || err_exit "arithmetic for syntax error" \
"(expected match of *$(printf %q "$exp"), got $(printf %q "$got"))"
exp=': `))'\'' unexpected'
for t in 'for((i=0,i<10,i++))' 'for(())' 'for((;))' 'for((0;))'
do got=$(set +x; (ulimit -c 0; eval "$t; do :; done") 2>&1)
[[ $got == *"$exp" ]] || err_exit "$t (expected match of *$(printf %q "$exp"), got $(printf %q "$got"))"
done

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

0 comments on commit 5aed2aa

Please sign in to comment.