From bf7e887b2442783ab52ddf9d1348c52344fc96f1 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Sun, 10 Jul 2022 15:22:13 +0200 Subject: [PATCH] tool_getparam: repair cleanarg Regression since 9e5669f. Make sure the "cleaning" of command line arguments is done on the original argv[] pointers. As a bonus, it also exits better on out of memory error. Reported-by: Litter White Fixes #9128 Closes #9130 --- src/tool_getparam.c | 61 +++++++++++++++++++++++++++++++-------------- src/tool_getparam.h | 3 ++- src/tool_paramhlp.c | 15 ----------- src/tool_paramhlp.h | 2 -- src/tool_parsecfg.c | 2 +- 5 files changed, 45 insertions(+), 38 deletions(-) diff --git a/src/tool_getparam.c b/src/tool_getparam.c index 9d425c846a..6423c8fe1d 100644 --- a/src/tool_getparam.c +++ b/src/tool_getparam.c @@ -487,7 +487,6 @@ GetFileAndPassword(char *nextarg, char **file, char **password) Curl_safefree(*password); *password = passphrase; } - cleanarg(nextarg); } /* Get a size parameter for '--limit-rate' or '--max-filesize'. @@ -542,8 +541,24 @@ static ParameterError GetSizeParameter(struct GlobalConfig *global, return PARAM_OK; } +static void cleanarg(char *str) +{ +#ifdef HAVE_WRITABLE_ARGV + /* now that GetStr has copied the contents of nextarg, wipe the next + * argument out so that the username:password isn't displayed in the + * system process list */ + if(str) { + size_t len = strlen(str); + memset(str, ' ', len); + } +#else + (void)str; +#endif +} + ParameterError getparameter(const char *flag, /* f or -long-flag */ char *nextarg, /* NULL if unset */ + char *clearthis, bool *usedarg, /* set to TRUE if the arg has been used */ struct GlobalConfig *global, @@ -675,7 +690,7 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */ break; case 'B': /* OAuth 2.0 bearer token */ GetStr(&config->oauth_bearer, nextarg); - cleanarg(nextarg); + cleanarg(clearthis); config->authtype |= CURLAUTH_BEARER; break; case 'c': /* connect-timeout */ @@ -1637,6 +1652,7 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */ case 'E': switch(subletter) { case '\0': /* certificate file */ + cleanarg(clearthis); GetFileAndPassword(nextarg, &config->cert, &config->key_passwd); break; case 'a': /* CA info PEM file */ @@ -1653,7 +1669,7 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */ break; case 'e': /* private key passphrase */ GetStr(&config->key_passwd, nextarg); - cleanarg(nextarg); + cleanarg(clearthis); break; case 'f': /* crypto engine */ GetStr(&config->engine, nextarg); @@ -1679,19 +1695,19 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */ break; case 'k': /* TLS username */ if(!(curlinfo->features & CURL_VERSION_TLSAUTH_SRP)) { - cleanarg(nextarg); + cleanarg(clearthis); return PARAM_LIBCURL_DOESNT_SUPPORT; } GetStr(&config->tls_username, nextarg); - cleanarg(nextarg); + cleanarg(clearthis); break; case 'l': /* TLS password */ if(!(curlinfo->features & CURL_VERSION_TLSAUTH_SRP)) { - cleanarg(nextarg); + cleanarg(clearthis); return PARAM_LIBCURL_DOESNT_SUPPORT; } GetStr(&config->tls_password, nextarg); - cleanarg(nextarg); + cleanarg(clearthis); break; case 'm': /* TLS authentication type */ if(curlinfo->features & CURL_VERSION_TLSAUTH_SRP) { @@ -1752,21 +1768,19 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */ break; case 'u': /* TLS username for proxy */ + cleanarg(clearthis); if(!(curlinfo->features & CURL_VERSION_TLSAUTH_SRP)) { - cleanarg(nextarg); return PARAM_LIBCURL_DOESNT_SUPPORT; } GetStr(&config->proxy_tls_username, nextarg); - cleanarg(nextarg); break; case 'v': /* TLS password for proxy */ + cleanarg(clearthis); if(!(curlinfo->features & CURL_VERSION_TLSAUTH_SRP)) { - cleanarg(nextarg); return PARAM_LIBCURL_DOESNT_SUPPORT; } GetStr(&config->proxy_tls_password, nextarg); - cleanarg(nextarg); break; case 'w': /* TLS authentication type for proxy */ @@ -1780,6 +1794,7 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */ break; case 'x': /* certificate file for proxy */ + cleanarg(clearthis); GetFileAndPassword(nextarg, &config->proxy_cert, &config->proxy_key_passwd); break; @@ -1798,7 +1813,7 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */ case '1': /* private key passphrase for proxy */ GetStr(&config->proxy_key_passwd, nextarg); - cleanarg(nextarg); + cleanarg(clearthis); break; case '2': /* ciphers for proxy */ @@ -2246,12 +2261,12 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */ case 'u': /* user:password */ GetStr(&config->userpwd, nextarg); - cleanarg(nextarg); + cleanarg(clearthis); break; case 'U': /* Proxy user:password */ GetStr(&config->proxyuserpwd, nextarg); - cleanarg(nextarg); + cleanarg(clearthis); break; case 'v': if(toggle) { @@ -2424,11 +2439,19 @@ ParameterError parse_args(struct GlobalConfig *global, int argc, following (URL) argument to start with -. */ stillflags = FALSE; else { - char *nextarg = (i < (argc - 1)) - ? curlx_convert_tchar_to_UTF8(argv[i + 1]) - : NULL; + char *nextarg = NULL; + char *clear = NULL; + if(i < (argc - 1)) { + nextarg = curlx_convert_tchar_to_UTF8(argv[i + 1]); + if(!nextarg) { + curlx_unicodefree(orig_opt); + return PARAM_NO_MEM; + } + clear = argv[i + 1]; + } - result = getparameter(orig_opt, nextarg, &passarg, global, config); + result = getparameter(orig_opt, nextarg, clear, &passarg, + global, config); curlx_unicodefree(nextarg); config = global->last; @@ -2466,7 +2489,7 @@ ParameterError parse_args(struct GlobalConfig *global, int argc, bool used; /* Just add the URL please */ - result = getparameter("--url", orig_opt, &used, global, + result = getparameter("--url", orig_opt, NULL, &used, global, config); } diff --git a/src/tool_getparam.h b/src/tool_getparam.h index ef98335750..e351361235 100644 --- a/src/tool_getparam.h +++ b/src/tool_getparam.h @@ -54,7 +54,8 @@ typedef enum { struct GlobalConfig; struct OperationConfig; -ParameterError getparameter(const char *flag, char *nextarg, bool *usedarg, +ParameterError getparameter(const char *flag, char *nextarg, char *clearthis, + bool *usedarg, struct GlobalConfig *global, struct OperationConfig *operation); diff --git a/src/tool_paramhlp.c b/src/tool_paramhlp.c index 71c738594d..db0d0fb00f 100644 --- a/src/tool_paramhlp.c +++ b/src/tool_paramhlp.c @@ -116,21 +116,6 @@ ParameterError file2memory(char **bufp, size_t *size, FILE *file) return PARAM_OK; } -void cleanarg(char *str) -{ -#ifdef HAVE_WRITABLE_ARGV - /* now that GetStr has copied the contents of nextarg, wipe the next - * argument out so that the username:password isn't displayed in the - * system process list */ - if(str) { - size_t len = strlen(str); - memset(str, ' ', len); - } -#else - (void)str; -#endif -} - /* * Parse the string and write the long in the given address. Return PARAM_OK * on success, otherwise a parameter specific error enum. diff --git a/src/tool_paramhlp.h b/src/tool_paramhlp.h index 297490b57c..ec44e2df4d 100644 --- a/src/tool_paramhlp.h +++ b/src/tool_paramhlp.h @@ -31,8 +31,6 @@ ParameterError file2string(char **bufp, FILE *file); ParameterError file2memory(char **bufp, size_t *size, FILE *file); -void cleanarg(char *str); - ParameterError str2num(long *val, const char *str); ParameterError str2unum(long *val, const char *str); ParameterError oct2nummax(long *val, const char *str, long max); diff --git a/src/tool_parsecfg.c b/src/tool_parsecfg.c index a166757f86..34eb5daa9d 100644 --- a/src/tool_parsecfg.c +++ b/src/tool_parsecfg.c @@ -223,7 +223,7 @@ int parseconfig(const char *filename, struct GlobalConfig *global) #ifdef DEBUG_CONFIG fprintf(stderr, "PARAM: \"%s\"\n",(param ? param : "(null)")); #endif - res = getparameter(option, param, &usedarg, global, operation); + res = getparameter(option, param, NULL, &usedarg, global, operation); operation = global->last; if(!res && param && *param && !usedarg)