From: Daniel Stenberg Date: Mon, 20 Oct 2025 20:46:56 +0000 (+0200) Subject: tool_parsecfg: detect and error on recursive --config use X-Git-Tag: curl-8_17_0~155 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e198618dea14231488005b53c7800d4e80e2dce;p=thirdparty%2Fcurl.git 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 --- diff --git a/src/tool_getparam.c b/src/tool_getparam.c index 0cff5b558d..5624d3cd76 100644 --- a/src/tool_getparam.c +++ b/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) { diff --git a/src/tool_getparam.h b/src/tool_getparam.h index 6b97b9c11f..6b37cc50eb 100644 --- a/src/tool_getparam.h +++ b/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, diff --git a/src/tool_helpers.c b/src/tool_helpers.c index fbf3f1c932..b36bd4af1d 100644 --- a/src/tool_helpers.c +++ b/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: diff --git a/src/tool_operate.c b/src/tool_operate.c index 0c03114d40..1c00351729 100644 --- a/src/tool_operate.c +++ b/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)) { diff --git a/src/tool_parsecfg.c b/src/tool_parsecfg.c index 5cf3527ce2..03f9930c5d 100644 --- a/src/tool_parsecfg.c +++ b/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; } diff --git a/src/tool_parsecfg.h b/src/tool_parsecfg.h index 46977d8edd..3aad9bdd9c 100644 --- a/src/tool_parsecfg.h +++ b/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 */ diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 9984e92839..21d20151d3 100644 --- a/tests/data/Makefile.am +++ b/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 \ \ diff --git a/tests/data/test2080 b/tests/data/test2080 index d49bd5c453..44d16b6ee3 100644 --- a/tests/data/test2080 +++ b/tests/data/test2080 @@ -30,7 +30,7 @@ config file with overly long option # the used option in the config file is too long -26 +2 diff --git a/tests/data/test462 b/tests/data/test462 index fdbce3b462..3fbcae7313 100644 --- a/tests/data/test462 +++ b/tests/data/test462 @@ -30,7 +30,7 @@ http://%HOSTIP:%HTTPPORT/%TESTNUMBER -K %LOGDIR/cmd # Verify data after the test has been "shot" -26 +2 diff --git a/tests/data/test774 b/tests/data/test774 new file mode 100644 index 0000000000..758ad0de8d --- /dev/null +++ b/tests/data/test774 @@ -0,0 +1,25 @@ + + + +--config + + + + + +config file recursively including itself + + +--config %LOGDIR/cmd + + +http://%HOSTIP:%HTTPPORT/ -K %LOGDIR/cmd + + + + + +2 + + +