From f8f2c4b60835dd5d37a02f8dc0fe62ab74e4da94 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Fri, 5 Mar 2021 21:55:25 +0000 Subject: [PATCH] Remove obsolete quote balancing hack The old Bourne shell failed to check for closing quotes and command substitution backticks when encountering end-of-file in a parser context (such as a script). ksh93 implemented a hack for partial compatibility with this bug, tolerating unbalanced quotes and backticks in backtick command subsitutions, 'eval', and command line invocation '-c' scripts only. This hack became broken for backtick command substitutions in fe20311f/350b52ea as a memory leak was fixed by adding a newline to the stack at the end of the command substitution. That extra newline becomes part of any string whose quotes are not properly terminated, causing problems such as the one detailed here: https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01889.html $ touch abc $ echo `ls "abc` ls: abc : not found No other fix for the memory leak is known that doesn't cause other problems. (The alternative fix detailed in the referenced mailing list post causes a different corner-case regression.) Besides, the hack has always caused other corner case bugs as well: $ ksh -c '((i++' Actual: ksh: i++(: not found (If an external command 'i++(' existed, it would be run) Expect: ksh: syntax error at line 1: `(' unmatched $ ksh -c 'i=0; echo $((++i' Actual: (empty line; the arithmetic expansion is ignored) Expect: ksh: syntax error at line 1: `(' unmatched $ ksh -c 'echo $(echo "hi)' Actual: ksh: syntax error at line 1: `(' unmatched Expect: ksh: syntax error at line 1: `"' unmatched So, it's time to get rid of this hack. The old Bourne shell is dead and buried. No other shell tries to support this breakage. Tolerating syntax errors is just asking for strange side effects, inconsistent states, and corner case bugs. We should not want to do that. Old scripts that rely on this will just need to be fixed. src/cmd/ksh93/sh/lex.c: - struct lexdata: Remove 'char balance' member for remembering an unbalanced quote or backtick. - sh_lex(): Remove the back to remember and compensate for unbalanced quotes/backticks that was executed only if we were executing a script from a string, as opposed to a file. src/cmd/ksh93/COMPATIBILITY: - Note the change. Resolves: https://github.com/ksh93/ksh/issues/199 --- NEWS | 5 +++++ src/cmd/ksh93/COMPATIBILITY | 3 +++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/lex.c | 26 ++++---------------------- 4 files changed, 13 insertions(+), 23 deletions(-) diff --git a/NEWS b/NEWS index 24d4b67f6..0c2cdc32a 100644 --- a/NEWS +++ b/NEWS @@ -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-05: + +- Unbalanced quotes and backticks now correctly produce a syntax error + in -c scripts, 'eval', and backtick-style command substitutions. + 2021-03-04: - Fixed an arbitrary command execution vulnerability that occurred when diff --git a/src/cmd/ksh93/COMPATIBILITY b/src/cmd/ksh93/COMPATIBILITY index e70aa2c83..7c7585c9e 100644 --- a/src/cmd/ksh93/COMPATIBILITY +++ b/src/cmd/ksh93/COMPATIBILITY @@ -89,6 +89,9 @@ For more details, see the NEWS file and for complete details, see the git log. 15. 'command -x' now always runs an external command, bypassing built-ins. +16. Unbalanced quotes and backticks now correctly produce a syntax error + in -c scripts, 'eval', and backtick-style command substitutions. + ____________________________________________________________________________ KSH-93 VS. KSH-88 diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 80397a48a..6c5932688 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -20,7 +20,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2021-03-04" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2021-03-05" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ diff --git a/src/cmd/ksh93/sh/lex.c b/src/cmd/ksh93/sh/lex.c index 0fc46c4d6..1b7cee923 100644 --- a/src/cmd/ksh93/sh/lex.c +++ b/src/cmd/ksh93/sh/lex.c @@ -92,7 +92,6 @@ struct lexdata char nested_tilde; char *docend; char noarg; - char balance; char warn; char message; char arith; @@ -277,7 +276,7 @@ Lex_t *sh_lexopen(Lex_t *lp, Shell_t *sp, int mode) lp->lexd.warn=1; if(!mode) { - lp->lexd.noarg = lp->lexd.level= lp->lexd.dolparen = lp->lexd.balance = 0; + lp->lexd.noarg = lp->lexd.level= lp->lexd.dolparen = 0; lp->lexd.nocopy = lp->lexd.docword = lp->lexd.nest = lp->lexd.paren = 0; lp->lexd.lex_state = lp->lexd.lastc=0; lp->lexd.docend = 0; @@ -326,7 +325,6 @@ int sh_lex(Lex_t* lp) Stk_t *stkp = shp->stk; int inlevel=lp->lexd.level, assignment=0, ingrave=0; int epatchar=0; - Sfio_t *sp; #if SHOPT_MULTIBYTE LEN=1; #endif /* SHOPT_MULTIBYTE */ @@ -397,7 +395,6 @@ int sh_lex(Lex_t* lp) fcseek(-LEN); goto breakloop; case S_EOF: - sp = fcfile(); if((n=lexfill(lp)) > 0) { fcseek(-1); @@ -446,16 +443,11 @@ int sh_lex(Lex_t* lp) c = LBRACE; break; case '"': case '`': case '\'': - lp->lexd.balance = c; break; } - if(sp && !(sfset(sp,0,0)&SF_STRING)) - { - lp->lasttok = c; - lp->token = EOFSYM; - sh_syntax(lp); - } - lp->lexd.balance = c; + lp->lasttok = c; + lp->token = EOFSYM; + sh_syntax(lp); } goto breakloop; case S_COM: @@ -1299,13 +1291,9 @@ int sh_lex(Lex_t* lp) } breakloop: if(lp->lexd.nocopy) - { - lp->lexd.balance = 0; return(0); - } if(lp->lexd.dolparen) { - lp->lexd.balance = 0; if(lp->lexd.docword) nested_here(lp); lp->lexd.message = (wordflags&ARG_MESSAGE); @@ -1318,12 +1306,6 @@ breakloop: lp->arg = (struct argnod*)stkseek(stkp,ARGVAL); if(n>0) sfwrite(stkp,state,n); - /* add balancing character if necessary */ - if(lp->lexd.balance) - { - sfputc(stkp,lp->lexd.balance); - lp->lexd.balance = 0; - } sfputc(stkp,0); stkseek(stkp,stktell(stkp)-1); state = stkptr(stkp,ARGVAL);