erlexec: Add checks that next argument is valid

The argument -- is silently added in a number of
places in order to seperate arguments from different
sources. When checking to see if an argument that
erlexec needs to handle is valid we therefore check
that it does not start with '--'.

Since `+S -1` is a valid argument we cannot make all
`-` and `+` values invalid, which is a shame but not
much we can do about that.

Closes #4624

2 files changed, 122 insertions, 74 deletions

diff --git a/erts/etc/common/erlexec.c b/erts/etc/common/erlexec.c
index 0efacd67a1..41edbd8636 100644
--- a/erts/etc/common/erlexec.c
+++ b/erts/etc/common/erlexec.c
@@ -404,6 +404,13 @@ static void add_boot_config(void)
 # define ADD_BOOT_CONFIG
 #endif
 
+#define NEXT_ARG_CHECK_NAMED(Option) \
+    do {                                                                \
+        if (i+1 >= argc || strncmp(argv[i+1], "--", 3) == 0)            \
+            usage(Option);                                              \
+    } while(0)
+
+#define NEXT_ARG_CHECK() NEXT_ARG_CHECK_NAMED(argv[i])
 
 #ifdef __WIN32__
 __declspec(dllexport) int win_erlexec(int argc, char **argv, HANDLE module, int windowed)
@@ -513,15 +520,11 @@ int main(int argc, char **argv)
 	    } else if (strcmp(argv[i], "-extra") == 0) {
 		break;
 	    } else if (strcmp(argv[i], "-emu_type") == 0) {
-		if (i + 1 >= argc) {
-                    usage(argv[i]);
-                }
+                NEXT_ARG_CHECK();
                 emu_type = argv[i+1];
                 i++;
 	    } else if (strcmp(argv[i], "-emu_flavor") == 0) {
-		if (i + 1 >= argc) {
-                    usage(argv[i]);
-                }
+                NEXT_ARG_CHECK();
                 emu_flavor = argv[i+1];
                 i++;
 	    }
@@ -610,8 +613,7 @@ int main(int argc, char **argv)
 			    error("Conflicting -boot options");
                         if (got_start_erl)
                             error("Conflicting -start_erl and -boot options");
-			if (i+1 >= argc)
-			    usage("-boot");
+                        NEXT_ARG_CHECK();
 			boot_script = strsave(argv[i+1]);
 			i++;
 		    }
@@ -635,8 +637,7 @@ int main(int argc, char **argv)
 		    else if (strcmp(argv[i], "-config") == 0){
 			if (got_start_erl)
 			    error("Conflicting -start_erl and -config options");
-			if (i+1 >= argc)
-			    usage("-config");
+                        NEXT_ARG_CHECK();
                         do {
                             config_script_cnt++;
                             config_scripts = erealloc(config_scripts,
@@ -647,8 +648,7 @@ int main(int argc, char **argv)
 		    }
 #endif
                     else if (strcmp(argv[i], "-configfd") == 0) {
-                        if (i+1 >= argc)
- 			    usage("-configfd");
+                        NEXT_ARG_CHECK();
                         if ( strcmp(argv[i+1], "0") != 0 ) {
 			    add_arg(argv[i]);
                         } else {
@@ -685,13 +685,13 @@ int main(int argc, char **argv)
 		    } else if (strcmp(argv[i], "-emu_qouted_cmd_exit") == 0) {
 			print_qouted_cmd_exit = 1;
 		    } else if (strcmp(argv[i], "-env") == 0) { /* -env VARNAME VARVALUE */
-			if (i+2 >= argc)
-			    usage("-env");
-			set_env(argv[i+1], argv[i+2]);
-			i += 2;
+                        NEXT_ARG_CHECK();
+                        i += 1;
+                        NEXT_ARG_CHECK_NAMED("-env");
+			set_env(argv[i], argv[i+1]);
+			i += 1;
 		    } else if (strcmp(argv[i], "-epmd") == 0) {
-			if (i+1 >= argc)
-			    usage("-epmd");
+                        NEXT_ARG_CHECK();
 			epmd_prog = argv[i+1];
 			++i;
 		    } else {
@@ -731,8 +731,7 @@ int main(int argc, char **argv)
 
 		  case 'n':
 		    if (strcmp(argv[i], "-name") == 0) { /* -name NAME */
-			if (i+1 >= argc)
-			    usage("-name");
+                        NEXT_ARG_CHECK();
 
 			/*
 			 * Note: Cannot use add_args() here, due to non-defined
@@ -758,8 +757,7 @@ int main(int argc, char **argv)
 
 		  case 's':	/* -sname NAME */
 		    if (strcmp(argv[i], "-sname") == 0) {
-			if (i+1 >= argc)
-			    usage("-sname");
+                        NEXT_ARG_CHECK();
 			add_arg(argv[i]);
 			add_arg(argv[i+1]);
 			isdistributed = 1;
@@ -767,6 +765,7 @@ int main(int argc, char **argv)
 		    }
 #ifdef __WIN32__
 		    else if (strcmp(argv[i], "-service_event") == 0) {
+                        NEXT_ARG_CHECK();
 			add_arg(argv[i]);
 			add_arg(argv[i+1]);
 			i++;
@@ -780,8 +779,7 @@ int main(int argc, char **argv)
 		    }
 #endif
 		    else if (strcmp(argv[i], "-start_epmd") == 0) {
-			if (i+1 >= argc)
-			    usage("-start_epmd");
+                        NEXT_ARG_CHECK();
 
 			if (strcmp(argv[i+1], "true") == 0) {
 			    /* The default */
@@ -833,8 +831,7 @@ int main(int argc, char **argv)
 		  case 'K':
 		      if (argv[i][2] != '\0')
 			  goto the_default;
-		      if (i+1 >= argc)
-			  usage(argv[i]);
+                      NEXT_ARG_CHECK();
 		      argv[i][0] = '-';
 		      add_Eargs(argv[i]);
 		      add_Eargs(argv[i+1]);
@@ -844,6 +841,7 @@ int main(int argc, char **argv)
                       if (argv[i][2] == 'O' && (argv[i][3] == 't' || argv[i][3] == 'p')) {
                           if (argv[i][4] != '\0')
                               goto the_default;
+                          NEXT_ARG_CHECK();
                           argv[i][0] = '-';
                           add_Eargs(argv[i]);
                           add_Eargs(argv[i+1]);
@@ -854,6 +852,7 @@ int main(int argc, char **argv)
                           (argv[i][4] == 't' || argv[i][4] == 'p')) {
                           if (argv[i][5] != '\0')
                               goto the_default;
+                          NEXT_ARG_CHECK();
                           argv[i][0] = '-';
                           add_Eargs(argv[i]);
                           add_Eargs(argv[i+1]);
@@ -863,9 +862,7 @@ int main(int argc, char **argv)
                       usage(argv[i]);
                       break;
                   case 'J':
-                      if (i + 1 >= argc) {
-                          usage(argv[i]);
-                      }
+                      NEXT_ARG_CHECK();
                       argv[i][0] = '-';
                       add_Eargs(argv[i]);
                       add_Eargs(argv[i+1]);
@@ -889,8 +886,7 @@ int main(int argc, char **argv)
 		      }
 		      else if (argv[i][2] != '\0')
 			  goto the_default;
-		      if (i+1 >= argc)
-			  usage(argv[i]);
+                      NEXT_ARG_CHECK();
 		      argv[i][0] = '-';
 		      add_Eargs(argv[i]);
 		      add_Eargs(argv[i+1]);
@@ -909,17 +905,17 @@ int main(int argc, char **argv)
 			}
 		      }
 		      if (i+1 < argc) {
-			if ((argv[i+1][0] != '-') &&
-			    (argv[i+1][0] != '+')) {
-			  if (argv[i+1][0] == 'i') {
-			    add_Eargs(argv[i]);
-			    add_Eargs(argv[i+1]);
-			    i++;
-			    break;
-			  } else {
-			    usage(argv[i]);
+                          if (argv[i+1][1] == '\0') {
+                              if ((argv[i+1][0] == 'i') ||
+                                  (argv[i+1][0] == 'c') ||
+                                  (argv[i+1][0] == 'd')
+                                  ) {
+                                  add_Eargs(argv[i]);
+                                  add_Eargs(argv[i+1]);
+                                  i++;
+                                  break;
+                              }
 			  }
-			}
 		      }
 		      add_Eargs(argv[i]);
 		      break;
@@ -946,10 +942,7 @@ int main(int argc, char **argv)
 						plusM_au_alloc_switches))
 			  || is_one_of_strings(&argv[i][2],
 					       plusM_other_switches)) {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
@@ -963,10 +956,7 @@ int main(int argc, char **argv)
 		      if (!is_one_of_strings(&argv[i][2], plush_val_switches)) {
 			  goto the_default;
 		      } else {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
@@ -978,10 +968,7 @@ int main(int argc, char **argv)
 					     plusr_val_switches))
 			  goto the_default;
 		      else {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
@@ -993,10 +980,7 @@ int main(int argc, char **argv)
 					     pluss_val_switches))
 			  goto the_default;
 		      else {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
@@ -1006,8 +990,7 @@ int main(int argc, char **argv)
 		  case 'p':
 		      if (argv[i][2] != 'c' || argv[i][3] != '\0')
 			  goto the_default;
-		      if (i+1 >= argc)
-			  usage(argv[i]);
+                      NEXT_ARG_CHECK();
 		      argv[i][0] = '-';
 		      add_Eargs(argv[i]);
 		      add_Eargs(argv[i+1]);
@@ -1017,10 +1000,7 @@ int main(int argc, char **argv)
 		      if (!is_one_of_strings(&argv[i][2], plusz_val_switches)) {
 			  goto the_default;
 		      } else {
-			  if (i+1 >= argc
-			      || argv[i+1][0] == '-'
-			      || argv[i+1][0] == '+')
-			      usage(argv[i]);
+                          NEXT_ARG_CHECK();
 			  argv[i][0] = '-';
 			  add_Eargs(argv[i]);
 			  add_Eargs(argv[i+1]);
diff --git a/erts/test/erlexec_SUITE.erl b/erts/test/erlexec_SUITE.erl
index 5794f7e309..113f6e1ceb 100644
--- a/erts/test/erlexec_SUITE.erl
+++ b/erts/test/erlexec_SUITE.erl
@@ -27,14 +27,30 @@
 %%%-------------------------------------------------------------------
 -module(erlexec_SUITE).
 
--export([all/0, suite/0, init_per_testcase/2, end_per_testcase/2]).
+-export([all/0, suite/0, init_per_suite/1, end_per_suite/1,
+         init_per_testcase/2, end_per_testcase/2]).
 
 -export([args_file/1, evil_args_file/1, missing_args_file/1, env/1, args_file_env/1,
-         otp_7461/1, otp_7461_remote/1, argument_separation/1,
+         otp_7461/1, otp_7461_remote/1, argument_separation/1, argument_with_option/1,
          zdbbl_dist_buf_busy_limit/1]).
 
 -include_lib("common_test/include/ct.hrl").
 
+suite() ->
+    [{ct_hooks,[ts_install_cth]},
+     {timetrap, {minutes, 1}}].
+
+all() -> 
+    [args_file, evil_args_file, missing_args_file, env, args_file_env,
+     otp_7461, argument_separation, argument_with_option, zdbbl_dist_buf_busy_limit].
+
+init_per_suite(Config) ->
+    [{suite_erl_flags, save_env()} | Config].
+
+end_per_suite(Config) ->
+    SavedEnv = proplists:get_value(suite_erl_flags, Config),
+    restore_env(SavedEnv).
+
 init_per_testcase(Case, Config) ->
     SavedEnv = save_env(),
     [{testcase, Case},{erl_flags_env, SavedEnv}|Config].
@@ -45,14 +61,6 @@ end_per_testcase(_Case, Config) ->
     cleanup_nodes(),
     ok.
 
-suite() ->
-    [{ct_hooks,[ts_install_cth]},
-     {timetrap, {minutes, 1}}].
-
-all() -> 
-    [args_file, evil_args_file, missing_args_file, env, args_file_env,
-     otp_7461, argument_separation, zdbbl_dist_buf_busy_limit].
-
 %% Test that plain first argument does not
 %% destroy -home switch [OTP-8209] or interact with environments
 argument_separation(Config) when is_list(Config) ->
@@ -114,6 +122,66 @@ flush() ->
             ok
     end.
 
+%% Test that giving an invalid argument to an option that needs
+%% an argument will always fail.
+argument_with_option(Config) when is_list(Config) ->
+    ErlSingle = ["emu_flavor", "emu_type", "configfd", "epmd",
+                 "name", "sname", "start_epmd"] ++
+        case os:type() of
+            {win32,_} -> ["boot","config","service_event"];
+            _ -> []
+        end,
+
+    MissingCheck =
+        fun(CmdLine, Prefix, Postfix) ->
+                InvalidArg = emu_args("-s init stop " ++ Prefix ++ CmdLine ++ Postfix,
+                                      [return_output]),
+                case re:run(InvalidArg, "Missing argument\\(s\\) for '\\" ++ Prefix ++ CmdLine ++ "'.") of
+                    nomatch ->
+                        ct:log("Output: ~ts",[InvalidArg]),
+                        ct:fail("Failed to fail on ~ts",[Prefix ++ CmdLine ++ Postfix]);
+                    {match,_} ->
+                        ok
+                end
+        end,
+    
+    [begin
+         MissingCheck(CmdLine,"-",""),
+
+         %% We test what happens when ERL_FLAGS is set
+         os:putenv("ERL_FLAGS","-test"),
+         MissingCheck(CmdLine,"-", ""),
+         os:unsetenv("ERL_FLAGS")
+     end || CmdLine <- ErlSingle],
+
+    EmuSingle = ["a", "A", "C", "e", "i", "n", "P", "Q", "t",
+                 "T", "R", "W", "K", "IOt", "IOp", "IOPt", "IOPp",
+                 "J", "SP", "SDcpu", "SDPcpu", "SDio",
+                 "hms", "rg", "sbt", "zdbbl"],
+
+    [begin
+         MissingCheck(CmdLine,"+",""),
+
+         %% We test what happens when ERL_FLAGS is set
+         os:putenv("ERL_FLAGS","-test"),
+         MissingCheck(CmdLine,"+", ""),
+         os:unsetenv("ERL_FLAGS")
+     end || CmdLine <- EmuSingle],
+
+    ErlDouble = ["env"],
+    
+    [begin
+         MissingCheck(CmdLine,"-",""),
+         MissingCheck(CmdLine,"-"," a"),
+
+         %% We test what happens when ERL_FLAGS is set
+         os:putenv("ERL_FLAGS","-test"),
+         MissingCheck(CmdLine,"-", ""),
+         MissingCheck(CmdLine,"-", " a"),
+         os:unsetenv("ERL_FLAGS")
+     end || CmdLine <- ErlDouble],
+    ok.
+
 args_file(Config) when is_list(Config) ->
     AFN1 = privfile("1", Config),
     AFN2 = privfile("2", Config),
@@ -445,7 +513,7 @@ emu_args(CmdLineArgs, Opts) ->
     io:format("CmdLineArgs = ~ts, Opts = ~p~n", [CmdLineArgs, Opts]),
     {ok,[[Erl]]} = init:get_argument(progname),
     EmuCL = os:cmd(Erl ++ " -emu_qouted_cmd_exit " ++ CmdLineArgs),
-    ct:pal("EmuCL = ~ts", [EmuCL]),
+%    ct:pal("EmuCL = ~ts", [EmuCL]),
     case lists:member(return_output, Opts) of
         true ->
             EmuCL;