From 034f8623610a139010ee451d0ae81689c064209c Mon Sep 17 00:00:00 2001
From: Dmitry Goncharov <dgoncharov@users.sf.net>
Date: Sun, 4 Aug 2024 15:13:44 -0400
Subject: [PATCH] [SV 66037] Avoid hang/crash from MAKEFLAGS=... on command
 line

Make enters an infinite loop when some option and MAKEFLAGS=<value>
are specified on the command line.  For example,
    $ make -r MAKEFLAGS=hello=world

If decode_switches() runs handle_non_switch_argument() from within
the getopt() loop, it would recursively call decode_switches() to
enter a new getopt() loop, corrupting the state of the outer loop.

* src/main.c (decode_switches): Save up non-option arguments and run
handle_non_switch_argument() only after we're done with getopt().
* tests/scripts/variables/MAKEFLAGS: Add tests.
---
 src/main.c                        | 35 +++++++++++++++++++++++--------
 tests/scripts/variables/MAKEFLAGS | 23 ++++++++++++++++++++
 2 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/src/main.c b/src/main.c
index bf3829a0..0367f08b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -3108,8 +3108,20 @@ decode_switches (int argc, const char **argv, enum variable_origin origin)
   int bad = 0;
   struct command_switch *cs;
   struct stringlist *sl;
+  struct stringlist targets;
   int c;
   unsigned int found_wait = 0;
+  const char **a;
+
+  /* This is for safety/double-checking.  */
+  static int using_getopt = 0;
+  assert (using_getopt == 0);
+  using_getopt = 1;
+
+  /* Get enough space for all the arguments, just in case.  */
+  targets.max = argc + 1;
+  targets.list = alloca (targets.max * sizeof (const char **));
+  targets.idx = 0;
 
   /* getopt does most of the parsing for us.
      First, get its vectors set up.  */
@@ -3138,14 +3150,9 @@ decode_switches (int argc, const char **argv, enum variable_origin origin)
            see all he did wrong.  */
         bad = 1;
       else if (c == 1)
-        {
-          /* An argument not starting with a dash.  */
-          const unsigned int prior_found_wait = found_wait;
-          found_wait = handle_non_switch_argument (coptarg, origin);
-          if (prior_found_wait && lastgoal)
-            /* If the argument before this was .WAIT, wait here.  */
-            lastgoal->wait_here = 1;
-        }
+        /* An argument not starting with a dash.  Defer handling until later,
+           since handle_non_switch_argument() might corrupt getopt().  */
+        targets.list[targets.idx++] = coptarg;
       else
         /* An option starting with a dash.  */
         for (cs = switches; cs->c != '\0'; ++cs)
@@ -3339,9 +3346,19 @@ decode_switches (int argc, const char **argv, enum variable_origin origin)
      to be returned in order, this only happens when there is a "--"
      argument to prevent later arguments from being options.  */
   while (optind < argc)
+    targets.list[targets.idx++] = argv[optind++];
+  targets.list[targets.idx] = NULL;
+
+  /* Cannot call getopt below this line.  */
+  using_getopt = 0;
+
+  /* handle_non_switch_argument() can only be called after getopt is done;
+     if one of the arguments is MAKEFLAGS=<value> then it will recurse here
+     and call getopt() again, corrupting the state if the outer method.  */
+  for (a = targets.list; *a; ++a)
     {
       const int prior_found_wait = found_wait;
-      found_wait = handle_non_switch_argument (argv[optind++], origin);
+      found_wait = handle_non_switch_argument (*a, origin);
       if (prior_found_wait && lastgoal)
         lastgoal->wait_here = 1;
     }
diff --git a/tests/scripts/variables/MAKEFLAGS b/tests/scripts/variables/MAKEFLAGS
index aab4a0e1..36b145fc 100644
--- a/tests/scripts/variables/MAKEFLAGS
+++ b/tests/scripts/variables/MAKEFLAGS
@@ -902,5 +902,28 @@ unlink('hello');
 
 rmdir('localtmp');
 
+# sv 66037. An infinite loop when MAKEFLAGS is specified on the command line.
+my @cli= ('-r MAKEFLAGS=-k hello=world',
+'-r MAKEFLAGS=-k hello=world MAKEFLAGS=-R',
+'-r MAKEFLAGS="-R -- hello=world MAKEFLAGS=-k"');
+for my $c (@cli) {
+run_make_test(q!
+$(info hello=$(hello))
+all:;
+!, $c, "hello=world\n#MAKE#: 'all' is up to date.\n");
+}
+
+run_make_test(q!
+$(info hello=$(hello))
+all:;
+!, '-r MAKEFLAGS="-R -- hello=world MAKEFLAGS=hello=bye"',
+"hello=bye\n#MAKE#: 'all' is up to date.\n");
+
+run_make_test(q!
+$(info hello=$(hello))
+all:;
+!, '-r MAKEFLAGS="-R -- hello=world MAKEFLAGS=-s"',
+"hello=world\n");
+
 # This tells the test driver that the perl test script executed properly.
 1;