Module Name: src
Committed By: rillig
Date: Mon May 8 10:24:07 UTC 2023
Modified Files:
src/usr.bin/make: for.c
src/usr.bin/make/unit-tests: directive-for-errors.exp
directive-for-errors.mk directive-for-escape.exp
directive-for-escape.mk directive-for.exp directive-for.mk
Log Message:
make: disallow characters like '$' in variable names in .for loops
Fixes PR 53146.
To generate a diff of this commit:
cvs rdiff -u -r1.172 -r1.173 src/usr.bin/make/for.c
cvs rdiff -u -r1.2 -r1.3 src/usr.bin/make/unit-tests/directive-for-errors.exp
cvs rdiff -u -r1.3 -r1.4 src/usr.bin/make/unit-tests/directive-for-errors.mk
cvs rdiff -u -r1.17 -r1.18 \
src/usr.bin/make/unit-tests/directive-for-escape.exp \
src/usr.bin/make/unit-tests/directive-for.mk
cvs rdiff -u -r1.16 -r1.17 \
src/usr.bin/make/unit-tests/directive-for-escape.mk
cvs rdiff -u -r1.14 -r1.15 src/usr.bin/make/unit-tests/directive-for.exp
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/usr.bin/make/for.c
diff -u src/usr.bin/make/for.c:1.172 src/usr.bin/make/for.c:1.173
--- src/usr.bin/make/for.c:1.172 Mon May 8 09:01:20 2023
+++ src/usr.bin/make/for.c Mon May 8 10:24:07 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: for.c,v 1.172 2023/05/08 09:01:20 rillig Exp $ */
+/* $NetBSD: for.c,v 1.173 2023/05/08 10:24:07 rillig Exp $ */
/*
* Copyright (c) 1992, The Regents of the University of California.
@@ -58,7 +58,7 @@
#include "make.h"
/* "@(#)for.c 8.1 (Berkeley) 6/6/93" */
-MAKE_RCSID("$NetBSD: for.c,v 1.172 2023/05/08 09:01:20 rillig Exp $");
+MAKE_RCSID("$NetBSD: for.c,v 1.173 2023/05/08 10:24:07 rillig Exp $");
typedef struct ForLoop {
@@ -139,6 +139,13 @@ ForLoop_Details(ForLoop *f)
}
static bool
+IsValidInVarname(char c)
+{
+ return c != '$' && c != ':' && c != '\\' &&
+ c != '(' && c != '{' && c != ')' && c != '}';
+}
+
+static bool
ForLoop_ParseVarnames(ForLoop *f, const char **pp)
{
const char *p = *pp;
@@ -152,12 +159,15 @@ ForLoop_ParseVarnames(ForLoop *f, const
return false;
}
- /*
- * XXX: This allows arbitrary variable names;
- * see directive-for.mk.
- */
- for (len = 1; p[len] != '\0' && !ch_isspace(p[len]); len++)
- continue;
+ for (len = 0; p[len] != '\0' && !ch_isspace(p[len]); len++) {
+ if (!IsValidInVarname(p[len])) {
+ Parse_Error(PARSE_FATAL,
+ "invalid character '%c' "
+ "in .for loop variable name",
+ p[len]);
+ return false;
+ }
+ }
if (len == 2 && p[0] == 'i' && p[1] == 'n') {
p += 2;
Index: src/usr.bin/make/unit-tests/directive-for-errors.exp
diff -u src/usr.bin/make/unit-tests/directive-for-errors.exp:1.2 src/usr.bin/make/unit-tests/directive-for-errors.exp:1.3
--- src/usr.bin/make/unit-tests/directive-for-errors.exp:1.2 Tue Feb 23 15:19:41 2021
+++ src/usr.bin/make/unit-tests/directive-for-errors.exp Mon May 8 10:24:07 2023
@@ -4,19 +4,20 @@ make: "directive-for-errors.mk" line 9:
make: "directive-for-errors.mk" line 19: Unknown directive "for"
make: "directive-for-errors.mk" line 20: warning:
make: "directive-for-errors.mk" line 21: for-less endfor
-make: "directive-for-errors.mk" line 37: Dollar $ 1 1 and backslash 2 2 2.
-make: "directive-for-errors.mk" line 37: Dollar $ 3 3 and backslash 4 4 4.
-make: "directive-for-errors.mk" line 43: no iteration variables in for
-make: "directive-for-errors.mk" line 47: warning: Should not be reached.
-make: "directive-for-errors.mk" line 48: for-less endfor
-make: "directive-for-errors.mk" line 53: Wrong number of words (5) in .for substitution list with 3 variables
-make: "directive-for-errors.mk" line 64: missing `in' in for
-make: "directive-for-errors.mk" line 66: warning: Should not be reached.
-make: "directive-for-errors.mk" line 67: for-less endfor
-make: "directive-for-errors.mk" line 73: Unknown modifier "Z"
-make: "directive-for-errors.mk" line 74: warning: Should not be reached.
-make: "directive-for-errors.mk" line 74: warning: Should not be reached.
-make: "directive-for-errors.mk" line 74: warning: Should not be reached.
+make: "directive-for-errors.mk" line 34: invalid character '$' in .for loop variable name
+make: "directive-for-errors.mk" line 35: Dollar $ and backslash backslash backslash backslash.
+make: "directive-for-errors.mk" line 36: for-less endfor
+make: "directive-for-errors.mk" line 41: no iteration variables in for
+make: "directive-for-errors.mk" line 45: warning: Should not be reached.
+make: "directive-for-errors.mk" line 46: for-less endfor
+make: "directive-for-errors.mk" line 51: Wrong number of words (5) in .for substitution list with 3 variables
+make: "directive-for-errors.mk" line 63: missing `in' in for
+make: "directive-for-errors.mk" line 65: warning: Should not be reached.
+make: "directive-for-errors.mk" line 66: for-less endfor
+make: "directive-for-errors.mk" line 72: Unknown modifier "Z"
+make: "directive-for-errors.mk" line 73: warning: Should not be reached.
+make: "directive-for-errors.mk" line 73: warning: Should not be reached.
+make: "directive-for-errors.mk" line 73: warning: Should not be reached.
make: Fatal errors encountered -- cannot continue
make: stopped in unit-tests
exit status 1
Index: src/usr.bin/make/unit-tests/directive-for-errors.mk
diff -u src/usr.bin/make/unit-tests/directive-for-errors.mk:1.3 src/usr.bin/make/unit-tests/directive-for-errors.mk:1.4
--- src/usr.bin/make/unit-tests/directive-for-errors.mk:1.3 Sun Apr 4 10:13:09 2021
+++ src/usr.bin/make/unit-tests/directive-for-errors.mk Mon May 8 10:24:07 2023
@@ -1,4 +1,4 @@
-# $NetBSD: directive-for-errors.mk,v 1.3 2021/04/04 10:13:09 rillig Exp $
+# $NetBSD: directive-for-errors.mk,v 1.4 2023/05/08 10:24:07 rillig Exp $
#
# Tests for error handling in .for loops.
@@ -20,17 +20,15 @@
. warning ${i}
.endfor
-# As of 2020-12-31, the variable name can be an arbitrary word, it just needs
-# to be separated by whitespace. Even '$' and '\' are valid variable names,
-# which is not useful in practice.
+# Before for.c 1.173 from 2023-05-08, the variable name could be an arbitrary
+# word, it only needed to be separated by whitespace. Even '$' and '\' were
+# valid variable names, which was not useful in practice.
#
-# The '$$' is not replaced with the values '1' or '3' from the .for loop,
-# instead it is kept as-is, and when the .info directive expands its argument,
-# each '$$' gets replaced with a single '$'. The "long variable expression"
-# ${$} gets replaced though, even though this would be a parse error everywhere
-# outside a .for loop.
-#
-# The '\' on the other hand is treated as a normal variable name.
+# The '$$' was not replaced with the values '1' or '3' from the .for loop,
+# instead it was kept as-is, and when the .info directive expanded its
+# argument, each '$$' got replaced with a single '$'. The "long variable
+# expression" ${$} got replaced though, even though this would be a parse
+# error everywhere outside a .for loop.
${:U\$}= dollar # see whether the "variable" '$' is local
${:U\\}= backslash # see whether the "variable" '\' is local
.for $ \ in 1 2 3 4
@@ -38,8 +36,8 @@ ${:U\\}= backslash # see whether the "va
.endfor
# If there are no variables, there is no point in expanding the .for loop
-# since this would end up in an endless loop, each time consuming 0 of the
-# 3 values.
+# since this would end up in an endless loop, consuming 0 of the 3 values in
+# each iteration.
.for in 1 2 3
# XXX: This should not be reached. It should be skipped, as already done
# when the number of values is not a multiple of the number of variables,
@@ -49,7 +47,7 @@ ${:U\\}= backslash # see whether the "va
# There are 3 variables and 5 values. These 5 values cannot be split evenly
# among the variables, therefore the loop is not expanded at all, it is
-# rather skipped.
+# skipped instead.
.for a b c in 1 2 3 4 5
. warning Should not be reached.
.endfor
@@ -61,7 +59,8 @@ ${:U\\}= backslash # see whether the "va
.endfor
# A missing 'in' should parse the .for loop but skip the body.
-.for i : k
+# expect+1: missing `in' in for
+.for i over k
# XXX: As of 2020-12-31, this line is reached once.
. warning Should not be reached.
.endfor
Index: src/usr.bin/make/unit-tests/directive-for-escape.exp
diff -u src/usr.bin/make/unit-tests/directive-for-escape.exp:1.17 src/usr.bin/make/unit-tests/directive-for-escape.exp:1.18
--- src/usr.bin/make/unit-tests/directive-for-escape.exp:1.17 Sun Jun 12 16:09:21 2022
+++ src/usr.bin/make/unit-tests/directive-for-escape.exp Mon May 8 10:24:07 2023
@@ -49,14 +49,12 @@ For: end for 1
For: loop body:
. info ${:U\$}
make: "directive-for-escape.mk" line 121: $
-For: end for 1
-For: loop body:
-. info ${NUMBERS} ${:Ureplaced}
-make: "directive-for-escape.mk" line 129: one two three replaced
-For: end for 1
-For: loop body:
-. info ${:Ureplaced}
-make: "directive-for-escape.mk" line 139: replaced
+make: "directive-for-escape.mk" line 129: invalid character ':' in .for loop variable name
+make: "directive-for-escape.mk" line 130: one two three one three
+make: "directive-for-escape.mk" line 131: for-less endfor
+make: "directive-for-escape.mk" line 139: invalid character '}' in .for loop variable name
+make: "directive-for-escape.mk" line 140: one.c
+make: "directive-for-escape.mk" line 141: for-less endfor
For: end for 1
For: loop body:
. info . $$i: ${:Uinner}
@@ -69,46 +67,44 @@ For: loop body:
. info . $${i2}: ${i2}
. info . $${i,}: ${i,}
. info . adjacent: ${:Uinner}${:Uinner}${:Uinner:M*}${:Uinner}
-make: "directive-for-escape.mk" line 147: . $i: inner
-make: "directive-for-escape.mk" line 148: . ${i}: inner
-make: "directive-for-escape.mk" line 149: . ${i:M*}: inner
-make: "directive-for-escape.mk" line 150: . $(i): inner
-make: "directive-for-escape.mk" line 151: . $(i:M*): inner
-make: "directive-for-escape.mk" line 152: . ${i${:U}}: outer
-make: "directive-for-escape.mk" line 153: . ${i\}}: inner}
-make: "directive-for-escape.mk" line 154: . ${i2}: two
-make: "directive-for-escape.mk" line 155: . ${i,}: comma
-make: "directive-for-escape.mk" line 156: . adjacent: innerinnerinnerinner
-For: end for 1
-For: loop body:
-. info eight $$$$$$$$ and no cents.
-. info eight ${:Udollar}${:Udollar}${:Udollar}${:Udollar} and no cents.
-make: "directive-for-escape.mk" line 164: eight $$$$ and no cents.
-make: "directive-for-escape.mk" line 165: eight dollardollardollardollar and no cents.
-make: "directive-for-escape.mk" line 174: eight and no cents.
+make: "directive-for-escape.mk" line 148: . $i: inner
+make: "directive-for-escape.mk" line 149: . ${i}: inner
+make: "directive-for-escape.mk" line 150: . ${i:M*}: inner
+make: "directive-for-escape.mk" line 151: . $(i): inner
+make: "directive-for-escape.mk" line 152: . $(i:M*): inner
+make: "directive-for-escape.mk" line 153: . ${i${:U}}: outer
+make: "directive-for-escape.mk" line 154: . ${i\}}: inner}
+make: "directive-for-escape.mk" line 155: . ${i2}: two
+make: "directive-for-escape.mk" line 156: . ${i,}: comma
+make: "directive-for-escape.mk" line 157: . adjacent: innerinnerinnerinner
+make: "directive-for-escape.mk" line 165: invalid character '$' in .for loop variable name
+make: "directive-for-escape.mk" line 166: eight $$$$ and no cents.
+make: "directive-for-escape.mk" line 167: eight and no cents.
+make: "directive-for-escape.mk" line 168: for-less endfor
+make: "directive-for-escape.mk" line 176: eight and no cents.
For: end for 1
-make: "directive-for-escape.mk" line 181: newline in .for value
-make: "directive-for-escape.mk" line 181: newline in .for value
+make: "directive-for-escape.mk" line 183: newline in .for value
+make: "directive-for-escape.mk" line 183: newline in .for value
For: loop body:
. info short: ${:U" "}
. info long: ${:U" "}
-make: "directive-for-escape.mk" line 182: short: " "
-make: "directive-for-escape.mk" line 183: long: " "
+make: "directive-for-escape.mk" line 184: short: " "
+make: "directive-for-escape.mk" line 185: long: " "
For: end for 1
For: loop body:
For: end for 1
-Parse_PushInput: .for loop in directive-for-escape.mk, line 196
-make: "directive-for-escape.mk" line 196: newline in .for value
- in .for loop from directive-for-escape.mk:196 with i = "
+Parse_PushInput: .for loop in directive-for-escape.mk, line 198
+make: "directive-for-escape.mk" line 198: newline in .for value
+ in .for loop from directive-for-escape.mk:198 with i = "
"
For: loop body:
: ${:U" "}
SetFilenameVars: ${.PARSEDIR} = <some-dir> ${.PARSEFILE} = `directive-for-escape.mk'
-Parsing line 197: : ${:U" "}
+Parsing line 199: : ${:U" "}
ParseDependency(: " ")
-ParseEOF: returning to file directive-for-escape.mk, line 199
+ParseEOF: returning to file directive-for-escape.mk, line 201
SetFilenameVars: ${.PARSEDIR} = <some-dir> ${.PARSEFILE} = `directive-for-escape.mk'
-Parsing line 199: .MAKEFLAGS: -d0
+Parsing line 201: .MAKEFLAGS: -d0
ParseDependency(.MAKEFLAGS: -d0)
For: end for 1
For: loop body:
Index: src/usr.bin/make/unit-tests/directive-for.mk
diff -u src/usr.bin/make/unit-tests/directive-for.mk:1.17 src/usr.bin/make/unit-tests/directive-for.mk:1.18
--- src/usr.bin/make/unit-tests/directive-for.mk:1.17 Mon May 8 09:24:42 2023
+++ src/usr.bin/make/unit-tests/directive-for.mk Mon May 8 10:24:07 2023
@@ -1,4 +1,4 @@
-# $NetBSD: directive-for.mk,v 1.17 2023/05/08 09:24:42 rillig Exp $
+# $NetBSD: directive-for.mk,v 1.18 2023/05/08 10:24:07 rillig Exp $
#
# Tests for the .for directive.
#
@@ -144,18 +144,18 @@ EXPANSION${plus}= value
# except for whitespace. This allows for creative side effects. Hopefully
# nobody is misusing this "feature".
var= outer
+# expect+1: invalid character ':' in .for loop variable name
.for var:Q in value "quoted"
-. info ${var} ${var:Q} ${var:Q:Q}
+. info <${var}> <${var:Q}> <${var:Q:Q}>
.endfor
# The short expression '$$' is preserved, the long expressions are
# substituted.
-# expect+2: $ value value
+# expect+1: invalid character '$' in .for loop variable name
.for $ in value
-. info $$ ${$} $($)
+. info <$$> <${$}> <$($)>
.endfor
# From https://gnats.netbsd.org/53146.
-# expect+3: <> <> <a>
-# expect+2: <> <> <b>
+# expect+1: invalid character '$' in .for loop variable name
.for $(FOO) in a b
. info <$(FOO)> <$(foo)> <$($(FOO))>
.endfor
Index: src/usr.bin/make/unit-tests/directive-for-escape.mk
diff -u src/usr.bin/make/unit-tests/directive-for-escape.mk:1.16 src/usr.bin/make/unit-tests/directive-for-escape.mk:1.17
--- src/usr.bin/make/unit-tests/directive-for-escape.mk:1.16 Sun Jun 12 16:09:21 2022
+++ src/usr.bin/make/unit-tests/directive-for-escape.mk Mon May 8 10:24:07 2023
@@ -1,4 +1,4 @@
-# $NetBSD: directive-for-escape.mk,v 1.16 2022/06/12 16:09:21 rillig Exp $
+# $NetBSD: directive-for-escape.mk,v 1.17 2023/05/08 10:24:07 rillig Exp $
#
# Test escaping of special characters in the iteration values of a .for loop.
# These values get expanded later using the :U variable modifier, and this
@@ -121,20 +121,21 @@ VALUES= begin<$${UNDEF:Ufallback:N{{{}}
. info ${i}
.endfor
-# As of 2020-12-31, the name of the iteration variable can even contain
-# colons, which then affects variable expressions having this exact modifier.
-# This is clearly an unintended side effect of the implementation.
+# Before for.c 1.173 from 2023-05-08, the name of the iteration variable
+# could contain colons, which affected variable expressions having this exact
+# modifier. This possibility was neither intended nor documented.
NUMBERS= one two three
+# expect+1: invalid character ':' in .for loop variable name
.for NUMBERS:M*e in replaced
. info ${NUMBERS} ${NUMBERS:M*e}
.endfor
-# As of 2020-12-31, the name of the iteration variable can contain braces,
-# which gets even more surprising than colons, since it allows to replace
-# sequences of variable expressions. There is no practical use case for
-# this, though.
+# Before for.c 1.173 from 2023-05-08, the name of the iteration variable
+# could contain braces, which allowed to replace sequences of variable
+# expressions. This possibility was neither intended nor documented.
BASENAME= one
EXT= .c
+# expect+1: invalid character '}' in .for loop variable name
.for BASENAME}${EXT in replaced
. info ${BASENAME}${EXT}
.endfor
@@ -156,10 +157,11 @@ i,= comma
. info . adjacent: $i${i}${i:M*}$i
.endfor
-# The variable name can be a single '$' since there is no check on valid
-# variable names. ForLoop_SubstVarShort skips "stupid" variable names though,
-# but ForLoop_SubstVarLong naively parses the body of the loop, substituting
-# each '${$}' with an actual 'dollar'.
+# Before for.c 1.173 from 2023-05-08, the variable name could be a single '$'
+# since there was no check on valid variable names. ForLoop_SubstVarShort
+# skipped "stupid" variable names though, but ForLoop_SubstVarLong naively
+# parsed the body of the loop, substituting each '${$}' with an actual
+# '${:Udollar}'.
.for $ in dollar
. info eight $$$$$$$$ and no cents.
. info eight ${$}${$}${$}${$} and no cents.
Index: src/usr.bin/make/unit-tests/directive-for.exp
diff -u src/usr.bin/make/unit-tests/directive-for.exp:1.14 src/usr.bin/make/unit-tests/directive-for.exp:1.15
--- src/usr.bin/make/unit-tests/directive-for.exp:1.14 Mon May 8 09:24:42 2023
+++ src/usr.bin/make/unit-tests/directive-for.exp Mon May 8 10:24:07 2023
@@ -14,11 +14,15 @@ make: "directive-for.mk" line 140: {{}}
make: "directive-for.mk" line 140: )( )( )(
make: "directive-for.mk" line 140: ][ ][ ][
make: "directive-for.mk" line 140: }{ }{ }{
-make: "directive-for.mk" line 148: outer value value
-make: "directive-for.mk" line 148: outer "quoted" \"quoted\"
-make: "directive-for.mk" line 154: $ value value
-make: "directive-for.mk" line 160: <> <> <a>
-make: "directive-for.mk" line 160: <> <> <b>
+make: "directive-for.mk" line 148: invalid character ':' in .for loop variable name
+make: "directive-for.mk" line 149: <outer> <outer> <outer>
+make: "directive-for.mk" line 150: for-less endfor
+make: "directive-for.mk" line 154: invalid character '$' in .for loop variable name
+make: "directive-for.mk" line 155: <$> <> <>
+make: "directive-for.mk" line 156: for-less endfor
+make: "directive-for.mk" line 159: invalid character '$' in .for loop variable name
+make: "directive-for.mk" line 160: <> <> <>
+make: "directive-for.mk" line 161: for-less endfor
make: "directive-for.mk" line 166: Unknown modifier "Z"
make: "directive-for.mk" line 167: XXX: Not reached word1
make: "directive-for.mk" line 167: XXX: Not reached word3
