win: fixes [21b0629c81] - exec/open process pipe under windows (0-day vulnerability - insufficient escape)

2 files changed, 47 insertions, 38 deletions

diff --git a/tests/winPipe.test b/tests/winPipe.test
index f993e0c..c5d3846 100644
--- a/tests/winPipe.test
+++ b/tests/winPipe.test
@@ -323,22 +323,22 @@ test winpipe-7.2 {BuildCommandLine: null arguments} {win exec} {
 } {foo "" bar}
 test winpipe-7.3 {BuildCommandLine: dbl quote quoting #1} {win exec} {
     exec $env(COMSPEC) /c echo foo "\"" bar
-} {foo \" bar}
+} {foo "\"" bar}
 test winpipe-7.4 {BuildCommandLine: dbl quote quoting #2} {win exec} {
     exec $env(COMSPEC) /c echo foo {""} bar
-} {foo \"\" bar}
+} {foo "\"\"" bar}
 test winpipe-7.5 {BuildCommandLine: dbl quote quoting #3} {win exec} {
     exec $env(COMSPEC) /c echo foo "\" " bar
 } {foo "\" " bar}
 test winpipe-7.6 {BuildCommandLine: dbl quote quoting #4} {win exec} {
     exec $env(COMSPEC) /c echo foo {a="b"} bar
-} {foo a=\"b\" bar}
+} {foo "a=\"b\"" bar}
 test winpipe-7.7 {BuildCommandLine: dbl quote quoting #5} {win exec} {
     exec $env(COMSPEC) /c echo foo {a = "b"} bar
 } {foo "a = \"b\"" bar}
 test winpipe-7.8 {BuildCommandLine: dbl quote quoting #6} {win exec} {
     exec $env(COMSPEC) /c echo {"hello"} {""hello""} {"""hello"""} {"\"hello\""} {he llo} "he \" llo"
-} {\"hello\" \"\"hello\"\" \"\"\"hello\"\"\" \"\\\"hello\\\"\" "he llo" "he \" llo"}
+} {"\"hello\"" "\"\"hello\"\"" "\"\"\"hello\"\"\"" "\"\\\"hello\\\"\"" "he llo" "he \" llo"}
 test winpipe-7.9 {BuildCommandLine: N backslashes followed a quote rule #1} {win exec} {
     exec $env(COMSPEC) /c echo foo \\ bar
 } {foo \ bar}
diff --git a/win/tclWinPipe.c b/win/tclWinPipe.c
index e0d8c63..496e35b 100644
--- a/win/tclWinPipe.c
+++ b/win/tclWinPipe.c
@@ -1540,6 +1540,8 @@ BuildCommandLine(
     int quote, i;
     Tcl_DString ds;
 
+    const static char *specMetaChars = "&|^<>!%()";
+
     Tcl_DStringInit(&ds);
 
     /*
@@ -1567,52 +1569,59 @@ BuildCommandLine(
 	    Tcl_UniChar ch;
 	    for (start = arg; *start != '\0'; start += count) {
 		count = Tcl_UtfToUniChar(start, &ch);
-		if (Tcl_UniCharIsSpace(ch)) {	/* INTL: ISO space. */
+		if (Tcl_UniCharIsSpace(ch) ||
+		   (count == 1 && (*start=='"' || strchr(specMetaChars, *start)))
+		) {
+		    /* must be quoted */
 		    quote = 1;
 		    break;
 		}
 	    }
 	}
-	if (quote) {
+	if (!quote) {
+	    Tcl_DStringAppend(&ds, arg, -1);
+	} else {
 	    Tcl_DStringAppend(&ds, "\"", 1);
-	}
-	start = arg;
-	for (special = arg; ; ) {
-	    if ((*special == '\\') && (special[1] == '\\' ||
-		    special[1] == '"' || (quote && special[1] == '\0'))) {
-		Tcl_DStringAppend(&ds, start, (int) (special - start));
-		start = special;
-		while (1) {
-		    special++;
-		    if (*special == '"' || (quote && *special == '\0')) {
-			/*
-			 * N backslashes followed a quote -> insert N * 2 + 1
-			 * backslashes then a quote.
-			 */
-
-			Tcl_DStringAppend(&ds, start,
-				(int) (special - start));
-			break;
+	    start = arg;
+	    for (special = arg; *special != '\0'; ) {
+		if (*special == '\\' && (special[1] == '\\' || special[1] == '"' || special[1] == '\0')) {
+		    if (special > start) {
+			Tcl_DStringAppend(&ds, start, (int) (special - start));
 		    }
-		    if (*special != '\\') {
-			break;
+		    Tcl_DStringAppend(&ds, "\\\\", 2);
+		    start = ++special;
+		    continue;
+		}
+		if (*special == '"') {
+		    quote ^= 2; /* observe unpaired quotes */
+		    if (special > start) {
+			Tcl_DStringAppend(&ds, start, (int) (special - start));
 		    }
+		    Tcl_DStringAppend(&ds, "\\\"", 2);
+		    start = ++special;
+		    continue;
 		}
-		Tcl_DStringAppend(&ds, start, (int) (special - start));
-		start = special;
+		/* unpaired (escaped) quote causes special handling on meta-chars */
+		if ((quote & 2) && strchr(specMetaChars, *special)) {
+		    if (special > start) {
+			Tcl_DStringAppend(&ds, start, (int) (special - start));
+		    }
+		    /* unpaired - escape all special chars inside quotes "..." */
+		    Tcl_DStringAppend(&ds, "\"", 1);
+		    start = special;
+		    do {
+			special++;
+		    } while(*special && strchr(specMetaChars, *special));
+		    Tcl_DStringAppend(&ds, start, (int) (special - start));
+		    Tcl_DStringAppend(&ds, "\"", 1);
+		    start = special;
+		    continue;
+		}
+		special++;
 	    }
-	    if (*special == '"') {
+	    if (special > start) {
 		Tcl_DStringAppend(&ds, start, (int) (special - start));
-		Tcl_DStringAppend(&ds, "\\\"", 2);
-		start = special + 1;
 	    }
-	    if (*special == '\0') {
-		break;
-	    }
-	    special++;
-	}
-	Tcl_DStringAppend(&ds, start, (int) (special - start));
-	if (quote) {
 	    Tcl_DStringAppend(&ds, "\"", 1);
 	}
     }