tool_parsecfg: detect and error on recursive --config use

The config file parser now has a maximum level of inclusions allowed (5)
to detect and prevent recursive inclusions of itself leading to badness.

Bonus: clean up return code handling from the config parser.

Test 774 verifies
Closes #19168

src/tool_getparam.c

@@ -2167,7 +2167,8 @@ static ParameterError opt_bool(struct OperationConfig *config,
 /* opt_file handles file options */
 static ParameterError opt_file(struct OperationConfig *config,
                                const struct LongShort *a,
-                               const char *nextarg)
+                               const char *nextarg,
+                               int max_recursive)
 {
   ParameterError err = PARAM_OK;
   if((nextarg[0] == '-') && nextarg[1]) {
@@ -2189,9 +2190,13 @@ static ParameterError opt_file(struct OperationConfig *config,
     GetFileAndPassword(nextarg, &config->cert, &config->key_passwd);
     break;
   case C_CONFIG: /* --config */
-    if(parseconfig(nextarg)) {
-      errorf("cannot read config from '%s'", nextarg);
-      err = PARAM_READ_ERROR;
+    if(--max_recursive < 0) {
+      errorf("Max config file recursion level reached (%u)",
+             CONFIG_MAX_LEVELS);
+      err = PARAM_BAD_USE;
+    }
+    else {
+      err = parseconfig(nextarg, max_recursive);
     }
     break;
   case C_CRLFILE: /* --crlfile */
@@ -2831,7 +2836,8 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */
                             const char *nextarg,    /* NULL if unset */
                             bool *usedarg,    /* set to TRUE if the arg
                                                  has been used */
-                            struct OperationConfig *config)
+                            struct OperationConfig *config,
+                            int max_recursive)
 {
   const char *parse = NULL;
   bool longopt = FALSE;
@@ -2962,7 +2968,7 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */
               "Maybe ASCII was intended?", nextarg);
       }
       if(ARGTYPE(a->desc) == ARG_FILE)
-        err = opt_file(config, a, nextarg);
+        err = opt_file(config, a, nextarg, max_recursive);
       else /* if(ARGTYPE(a->desc) == ARG_STRG) */
         err = opt_string(config, a, nextarg);
       if(a->desc & ARG_CLEAR)
@@ -3020,7 +3026,8 @@ ParameterError parse_args(int argc, argv_item_t argv[])
           }
         }
 
-        result = getparameter(orig_opt, nextarg, &passarg, config);
+        result = getparameter(orig_opt, nextarg, &passarg, config,
+                              CONFIG_MAX_LEVELS);
 
         unicodefree(nextarg);
         config = global->last;
@@ -3056,7 +3063,7 @@ ParameterError parse_args(int argc, argv_item_t argv[])
       bool used;
 
       /* Just add the URL please */
-      result = getparameter("--url", orig_opt, &used, config);
+      result = getparameter("--url", orig_opt, &used, config, 0);
     }
 
     if(!result) {

src/tool_getparam.h

@@ -335,7 +335,6 @@ struct LongShort {
 
 typedef enum {
   PARAM_OK = 0,
-  PARAM_OPTION_AMBIGUOUS,
   PARAM_OPTION_UNKNOWN,
   PARAM_REQUIRES_PARAMETER,
   PARAM_BAD_USE,
@@ -358,6 +357,7 @@ typedef enum {
   PARAM_EXPAND_ERROR, /* --expand problem */
   PARAM_BLANK_STRING,
   PARAM_VAR_SYNTAX, /* --variable syntax error */
+  PARAM_RECURSION,
   PARAM_LAST
 } ParameterError;
 
@@ -368,7 +368,8 @@ const struct LongShort *findshortopt(char letter);
 
 ParameterError getparameter(const char *flag, const char *nextarg,
                             bool *usedarg,
-                            struct OperationConfig *config);
+                            struct OperationConfig *config,
+                            int max_recursive);
 
 #ifdef UNITTESTS
 void parse_cert_parameter(const char *cert_parameter,

src/tool_helpers.c

@@ -40,8 +40,6 @@ const char *param2text(ParameterError error)
     return "had unsupported trailing garbage";
   case PARAM_OPTION_UNKNOWN:
     return "is unknown";
-  case PARAM_OPTION_AMBIGUOUS:
-    return "is ambiguous";
   case PARAM_REQUIRES_PARAMETER:
     return "requires parameter";
   case PARAM_BAD_USE:

src/tool_operate.c

@@ -2221,7 +2221,7 @@ CURLcode operate(int argc, argv_item_t argv[])
   if((argc == 1) ||
      (first_arg && strncmp(first_arg, "-q", 2) &&
       strcmp(first_arg, "--disable"))) {
-    parseconfig(NULL); /* ignore possible failure */
+    parseconfig(NULL, CONFIG_MAX_LEVELS); /* ignore possible failure */
 
     /* If we had no arguments then make sure a url was specified in .curlrc */
     if((argc < 2) && (!global->first->url_list)) {

src/tool_parsecfg.c

@@ -81,11 +81,11 @@ static int unslashquote(const char *line, struct dynbuf *param)
 #define MAX_CONFIG_LINE_LENGTH (10*1024*1024)
 
 /* return 0 on everything-is-fine, and non-zero otherwise */
-int parseconfig(const char *filename)
+ParameterError parseconfig(const char *filename, int max_recursive)
 {
   FILE *file = NULL;
   bool usedarg = FALSE;
-  int rc = 0;
+  ParameterError err = PARAM_OK;
   struct OperationConfig *config = global->last;
   char *pathalloc = NULL;
 
@@ -96,7 +96,7 @@ int parseconfig(const char *filename)
       file = curlx_fopen(curlrc, FOPEN_READTEXT);
       if(!file) {
         free(curlrc);
-        return 1;
+        return PARAM_READ_ERROR;
       }
       filename = pathalloc = curlrc;
     }
@@ -133,12 +133,12 @@ int parseconfig(const char *filename)
     curlx_dyn_init(&pbuf, MAX_CONFIG_LINE_LENGTH);
     DEBUGASSERT(filename);
 
-    while(!rc && my_get_line(file, &buf, &fileerror)) {
+    while(!err && my_get_line(file, &buf, &fileerror)) {
       ParameterError res;
       lineno++;
       line = curlx_dyn_ptr(&buf);
       if(!line) {
-        rc = 1; /* out of memory */
+        err = PARAM_NO_MEM; /* out of memory */
         break;
       }
 
@@ -166,9 +166,11 @@ int parseconfig(const char *filename)
       /* the parameter starts here (unless quoted) */
       if(*line == '\"') {
         /* quoted parameter, do the quote dance */
-        rc = unslashquote(++line, &pbuf);
-        if(rc)
+        int rc = unslashquote(++line, &pbuf);
+        if(rc) {
+          err = PARAM_BAD_USE;
           break;
+        }
         param = curlx_dyn_len(&pbuf) ? curlx_dyn_ptr(&pbuf) : CURL_UNCONST("");
       }
       else {
@@ -206,7 +208,7 @@ int parseconfig(const char *filename)
 #ifdef DEBUG_CONFIG
       curl_mfprintf(tool_stderr, "PARAM: \"%s\"\n",(param ? param : "(null)"));
 #endif
-      res = getparameter(option, param, &usedarg, config);
+      res = getparameter(option, param, &usedarg, config, max_recursive);
       config = global->last;
 
       if(!res && param && *param && !usedarg)
@@ -240,10 +242,12 @@ int parseconfig(const char *filename)
            res != PARAM_VERSION_INFO_REQUESTED &&
            res != PARAM_ENGINES_REQUESTED &&
            res != PARAM_CA_EMBED_REQUESTED) {
-          const char *reason = param2text(res);
-          errorf("%s:%d: '%s' %s",
-                 filename, lineno, option, reason);
-          rc = (int)res;
+          /* only show error in the first level config call */
+          if(max_recursive == CONFIG_MAX_LEVELS) {
+            const char *reason = param2text(res);
+            errorf("%s:%d: '%s' %s", filename, lineno, option, reason);
+          }
+          err = res;
         }
       }
     }
@@ -252,13 +256,16 @@ int parseconfig(const char *filename)
     if(file != stdin)
       curlx_fclose(file);
     if(fileerror)
-      rc = 1;
+      err = PARAM_READ_ERROR;
   }
   else
-    rc = 1; /* could not open the file */
+    err = PARAM_READ_ERROR; /* could not open the file */
+
+  if((err == PARAM_READ_ERROR) && filename)
+    errorf("cannot read config from '%s'", filename);
 
   free(pathalloc);
-  return rc;
+  return err;
 }
 
 

src/tool_parsecfg.h

@@ -25,7 +25,9 @@
  ***************************************************************************/
 #include "tool_setup.h"
 
-int parseconfig(const char *filename);
+/* only allow this many levels of recursive --config use */
+#define CONFIG_MAX_LEVELS 5
+ParameterError parseconfig(const char *filename, int max_recursive);
 bool my_get_line(FILE *fp, struct dynbuf *db, bool *error);
 
 #endif /* HEADER_CURL_TOOL_PARSECFG_H */

tests/data/Makefile.am

@@ -110,7 +110,7 @@ test736 test737 test738 test739 test740 test741 test742 test743 test744 \
 test745 test746 test747 test748 test749 test750 test751 test752 test753 \
 test754 test755 test756 test757 test758 test759 test760 test761 test762 \
 test763 test764 test765 test766 test767 test768 test769 test770 test771 \
-test772 test773 \
+test772 test773 test774 \
 test780 test781 test782 test783 test784 test785 test786 test787 test788 \
 test789 test790 test791 test792 test793 test794         test796 test797 \
 \

tests/data/test2080

@@ -30,7 +30,7 @@ config file with overly long option
 <verify>
 # the used option in the config file is too long
 <errorcode>
-26
+2
 </errorcode>
 </verify>
 </testcase>

tests/data/test462

@@ -30,7 +30,7 @@ http://%HOSTIP:%HTTPPORT/%TESTNUMBER -K %LOGDIR/cmd
 # Verify data after the test has been "shot"
 <verify>
 <errorcode>
-26
+2
 </errorcode>
 </verify>
 </testcase>

tests/data/test774

@@ -0,0 +1,25 @@
+<testcase>
+<info>
+<keywords>
+--config
+</keywords>
+</info>
+
+<client>
+<name>
+config file recursively including itself
+</name>
+<file name="%LOGDIR/cmd">
+--config %LOGDIR/cmd
+</file>
+<command>
+http://%HOSTIP:%HTTPPORT/ -K %LOGDIR/cmd
+</command>
+</client>
+
+<verify>
+<errorcode>
+2
+</errorcode>
+</verify>
+</testcase>