Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
bin/ksh: recognise comments in command substitutions
To:
OpenBSD tech <tech@openbsd.org>
Date:
Sun, 19 Jul 2026 23:54:24 +0200

Download raw body.

Thread
  • Kirill A. Korinsky:

    bin/ksh: recognise comments in command substitutions

tech@,

when working on update for acme.sh I had discovered a bug in ksh.

The command substitution scanner treats every quote as syntax while
locating the closing parenthesis. It therefore mistakes an apostrophe
following a token boundary # for an opening quote, remains in its quote
state, and reports a spurious "no closing quote" error.

So, track whether the scanner is collecting a word and enter a comment
state only when # appears at a token boundary. Ignore quoting and
parentheses until the following newline; add a noexec regression
covering the here document form that exposed the bug.

Ok?

Index: bin/ksh/lex.c
===================================================================
RCS file: /home/cvs/src/bin/ksh/lex.c,v
diff -u -p -r1.80 lex.c
--- bin/ksh/lex.c	28 Apr 2024 16:43:15 -0000	1.80
+++ bin/ksh/lex.c	19 Jul 2026 21:47:35 -0000
@@ -43,6 +43,7 @@ struct lex_state {
 		struct scsparen_info {
 			int nparen;	/* count open parenthesis */
 			int csstate;	/* XXX remove */
+			int csword;	/* currently collecting a word */
 #define ls_scsparen ls_info.u_scsparen
 		} u_scsparen;
 
@@ -370,6 +371,7 @@ yylex(int cf)
 						PUSH_STATE(SCSPAREN);
 						statep->ls_scsparen.nparen = 1;
 						statep->ls_scsparen.csstate = 0;
+						statep->ls_scsparen.csword = 0;
 						*wp++ = COMSUB;
 					}
 				} else if (c == '{') /*}*/ {
@@ -482,26 +484,45 @@ yylex(int cf)
 				switch (c) {
 				case '(':
 					statep->ls_scsparen.nparen++;
+					statep->ls_scsparen.csword = 0;
 					break;
 				case ')':
 					statep->ls_scsparen.nparen--;
+					statep->ls_scsparen.csword = 0;
 					break;
 				case '\\':
 					statep->ls_scsparen.csstate = 1;
 					break;
 				case '"':
 					statep->ls_scsparen.csstate = 2;
+					statep->ls_scsparen.csword = 1;
 					break;
 				case '\'':
 					statep->ls_scsparen.csstate = 4;
+					statep->ls_scsparen.csword = 1;
 					ignore_backslash_newline++;
 					break;
+				case '#':
+					if (!statep->ls_scsparen.csword)
+						statep->ls_scsparen.csstate = 5;
+					break;
+				case ' ': case '\t': case '\n':
+				case '|': case '&': case ';': case '<': case '>':
+					statep->ls_scsparen.csword = 0;
+					break;
+				default:
+					statep->ls_scsparen.csword = 1;
+					break;
 				}
 				break;
 
 			case 1: /* backslash in normal mode */
+				statep->ls_scsparen.csstate = 0;
+				statep->ls_scsparen.csword = 1;
+				break;
+
 			case 3: /* backslash in double quotes */
-				--statep->ls_scsparen.csstate;
+				statep->ls_scsparen.csstate = 2;
 				break;
 
 			case 2: /* double quotes */
@@ -517,6 +538,13 @@ yylex(int cf)
 					ignore_backslash_newline--;
 				}
 				break;
+
+			case 5: /* comment */
+				if (c == '\n') {
+					statep->ls_scsparen.csstate = 0;
+					statep->ls_scsparen.csword = 0;
+				}
+				break;
 			}
 			if (statep->ls_scsparen.nparen == 0) {
 				POP_STATE();
@@ -555,6 +583,7 @@ yylex(int cf)
 						wp++;
 						statep->ls_scsparen.nparen = 1;
 						statep->ls_scsparen.csstate = 0;
+						statep->ls_scsparen.csword = 0;
 						state = statep->ls_state =
 						    SCSPAREN;
 					}
Index: regress/bin/ksh/heredoc.t
===================================================================
RCS file: /home/cvs/src/regress/bin/ksh/heredoc.t,v
diff -u -p -r1.1 heredoc.t
--- regress/bin/ksh/heredoc.t	2 Dec 2013 20:39:44 -0000	1.1
+++ regress/bin/ksh/heredoc.t	19 Jul 2026 21:38:27 -0000
@@ -142,6 +142,19 @@ expected-stdout:
 	done
 ---
 
+name: heredoc-9
+description:
+	Check that a quote in a here document inside a command substitution
+	does not affect parsing with noexec.
+arguments: !-n!
+script:
+	_x=$(
+	cat <<EOF
+	# '
+	EOF
+	)
+---
+
 name: heredoc-tmpfile-1
 description:
 	Check that heredoc temp files aren't removed too soon or too late.

-- 
wbr, Kirill