Skip to content

Commit

Permalink
Remove obsolete quote balancing hack
Browse files Browse the repository at this point in the history
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
fe20311/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: #199
  • Loading branch information
McDutchie committed Mar 5, 2021
1 parent 2215e03 commit f8f2c4b
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 23 deletions.
5 changes: 5 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/cmd/ksh93/COMPATIBILITY
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 @@ -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. */
Expand Down
26 changes: 4 additions & 22 deletions src/cmd/ksh93/sh/lex.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ struct lexdata
char nested_tilde;
char *docend;
char noarg;
char balance;
char warn;
char message;
char arith;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand All @@ -1318,12 +1306,6 @@ int sh_lex(Lex_t* lp)
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);
Expand Down

0 comments on commit f8f2c4b

Please sign in to comment.