Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/make make: disallow characters like '$' in variable ...



details:   https://anonhg.NetBSD.org/src/rev/32bfb24b9ba9
branches:  trunk
changeset: 374623:32bfb24b9ba9
user:      rillig <rillig%NetBSD.org@localhost>
date:      Mon May 08 10:24:07 2023 +0000

description:
make: disallow characters like '$' in variable names in .for loops

Fixes PR 53146.

diffstat:

 usr.bin/make/for.c                               |  26 ++++++--
 usr.bin/make/unit-tests/directive-for-errors.exp |  27 +++++----
 usr.bin/make/unit-tests/directive-for-errors.mk  |  29 +++++-----
 usr.bin/make/unit-tests/directive-for-escape.exp |  66 +++++++++++------------
 usr.bin/make/unit-tests/directive-for-escape.mk  |  26 +++++----
 usr.bin/make/unit-tests/directive-for.exp        |  14 +++-
 usr.bin/make/unit-tests/directive-for.mk         |  12 ++--
 7 files changed, 106 insertions(+), 94 deletions(-)

diffs (truncated from 363 to 300 lines):

diff -r 3ffc286a9ff7 -r 32bfb24b9ba9 usr.bin/make/for.c
--- a/usr.bin/make/for.c        Mon May 08 10:18:03 2023 +0000
+++ b/usr.bin/make/for.c        Mon May 08 10:24:07 2023 +0000
@@ -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;
diff -r 3ffc286a9ff7 -r 32bfb24b9ba9 usr.bin/make/unit-tests/directive-for-errors.exp
--- a/usr.bin/make/unit-tests/directive-for-errors.exp  Mon May 08 10:18:03 2023 +0000
+++ b/usr.bin/make/unit-tests/directive-for-errors.exp  Mon May 08 10:24:07 2023 +0000
@@ -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
diff -r 3ffc286a9ff7 -r 32bfb24b9ba9 usr.bin/make/unit-tests/directive-for-errors.mk
--- a/usr.bin/make/unit-tests/directive-for-errors.mk   Mon May 08 10:18:03 2023 +0000
+++ b/usr.bin/make/unit-tests/directive-for-errors.mk   Mon May 08 10:24:07 2023 +0000
@@ -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 @@
 .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 @@
 
 # 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 @@
 .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
diff -r 3ffc286a9ff7 -r 32bfb24b9ba9 usr.bin/make/unit-tests/directive-for-escape.exp
--- a/usr.bin/make/unit-tests/directive-for-escape.exp  Mon May 08 10:18:03 2023 +0000
+++ b/usr.bin/make/unit-tests/directive-for-escape.exp  Mon May 08 10:24:07 2023 +0000
@@ -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
+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
-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.
-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:
diff -r 3ffc286a9ff7 -r 32bfb24b9ba9 usr.bin/make/unit-tests/directive-for-escape.mk
--- a/usr.bin/make/unit-tests/directive-for-escape.mk   Mon May 08 10:18:03 2023 +0000
+++ b/usr.bin/make/unit-tests/directive-for-escape.mk   Mon May 08 10:24:07 2023 +0000
@@ -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



Home | Main Index | Thread Index | Old Index