From: Daniel Stenberg Date: Tue, 18 Nov 2025 08:04:42 +0000 (+0100) Subject: tool_getparam: verify that a file exists for some options X-Git-Tag: rc-8_18_0-1~244 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=88024c6d39db0a7797f12a386cad38ec1e155e32;p=thirdparty%2Fcurl.git tool_getparam: verify that a file exists for some options Passing the option as-is to libcurl is fine, but checking that the file exists allows the tool to better provide a helpful message. This now done for the following options: --cacert, --crlfile, --knownhosts, --netrc-file, --proxy-cacert amd --proxy-crlfile Bonus: bail out properly on OOM errors in the --cert parser. Reported-by: Wesley Moore Fixes #19583 Closes #19585 --- diff --git a/src/tool_getparam.c b/src/tool_getparam.c index bd90da1ec7..337ad33280 100644 --- a/src/tool_getparam.c +++ b/src/tool_getparam.c @@ -385,20 +385,21 @@ static const struct LongShort aliases[]= { #ifndef UNITTESTS static #endif -void parse_cert_parameter(const char *cert_parameter, - char **certname, - char **passphrase) +ParameterError parse_cert_parameter(const char *cert_parameter, + char **certname, + char **passphrase) { size_t param_length = strlen(cert_parameter); size_t span; const char *param_place = NULL; char *certname_place = NULL; + ParameterError err = PARAM_OK; *certname = NULL; *passphrase = NULL; /* most trivial assumption: cert_parameter is empty */ if(param_length == 0) - return; + return PARAM_BLANK_STRING; /* next less trivial: cert_parameter starts 'pkcs11:' and thus * looks like a RFC7512 PKCS#11 URI which can be used as-is. @@ -407,12 +408,16 @@ void parse_cert_parameter(const char *cert_parameter, if(curl_strnequal(cert_parameter, "pkcs11:", 7) || !strpbrk(cert_parameter, ":\\")) { *certname = strdup(cert_parameter); - return; + if(!*certname) + return PARAM_NO_MEM; + return PARAM_OK; } /* deal with escaped chars; find unescaped colon if it exists */ certname_place = malloc(param_length + 1); - if(!certname_place) - return; + if(!certname_place) { + err = PARAM_NO_MEM; + goto done; + } *certname = certname_place; param_place = cert_parameter; @@ -471,12 +476,19 @@ void parse_cert_parameter(const char *cert_parameter, param_place++; if(*param_place) { *passphrase = strdup(param_place); + if(!*passphrase) + err = PARAM_NO_MEM; } goto done; } } done: - *certname_place = '\0'; + if(err) { + tool_safefree(*certname); + } + else + *certname_place = '\0'; + return err; } /* Replace (in-place) '%20' by '+' according to RFC1866 */ @@ -507,18 +519,22 @@ static size_t replace_url_encoded_space_by_plus(char *url) return new_index; /* new size */ } -static void +static ParameterError GetFileAndPassword(const char *nextarg, char **file, char **password) { char *certname, *passphrase; + ParameterError err; /* nextarg is never NULL here */ - parse_cert_parameter(nextarg, &certname, &passphrase); - free(*file); - *file = certname; - if(passphrase) { - free(*password); - *password = passphrase; + err = parse_cert_parameter(nextarg, &certname, &passphrase); + if(!err) { + free(*file); + *file = certname; + if(passphrase) { + free(*password); + *password = passphrase; + } } + return err; } /* Get a size parameter for '--limit-rate' or '--max-filesize'. @@ -2164,6 +2180,19 @@ static ParameterError opt_bool(struct OperationConfig *config, return PARAM_OK; } +static ParameterError existingfile(char **store, + const struct LongShort *a, + const char *filename) +{ + struct_stat info; + if(curlx_stat(filename, &info)) { + errorf("The file '%s' provided to --%s does not exist", + filename, a->lname); + return PARAM_BAD_USE; + } + return getstr(store, filename, DENY_BLANK); +} + /* opt_file handles file options */ static ParameterError opt_file(struct OperationConfig *config, const struct LongShort *a, @@ -2181,13 +2210,13 @@ static ParameterError opt_file(struct OperationConfig *config, err = getstr(&config->unix_socket_path, nextarg, DENY_BLANK); break; case C_CACERT: /* --cacert */ - err = getstr(&config->cacert, nextarg, DENY_BLANK); + err = existingfile(&config->cacert, a, nextarg); break; case C_CAPATH: /* --capath */ err = getstr(&config->capath, nextarg, DENY_BLANK); break; case C_CERT: /* --cert */ - GetFileAndPassword(nextarg, &config->cert, &config->key_passwd); + err = GetFileAndPassword(nextarg, &config->cert, &config->key_passwd); break; case C_CONFIG: /* --config */ if(--max_recursive < 0) { @@ -2200,7 +2229,7 @@ static ParameterError opt_file(struct OperationConfig *config, } break; case C_CRLFILE: /* --crlfile */ - err = getstr(&config->crlfile, nextarg, DENY_BLANK); + err = existingfile(&config->crlfile, a, nextarg); break; case C_DUMP_HEADER: /* --dump-header */ err = getstr(&config->headerfile, nextarg, DENY_BLANK); @@ -2225,26 +2254,26 @@ static ParameterError opt_file(struct OperationConfig *config, err = getstr(&config->key, nextarg, DENY_BLANK); break; case C_KNOWNHOSTS: /* --knownhosts */ - err = getstr(&config->knownhosts, nextarg, DENY_BLANK); + err = existingfile(&config->knownhosts, a, nextarg); break; case C_NETRC_FILE: /* --netrc-file */ - err = getstr(&config->netrc_file, nextarg, DENY_BLANK); + err = existingfile(&config->netrc_file, a, nextarg); break; case C_OUTPUT: /* --output */ err = parse_output(config, nextarg); break; case C_PROXY_CACERT: /* --proxy-cacert */ - err = getstr(&config->proxy_cacert, nextarg, DENY_BLANK); + err = existingfile(&config->proxy_cacert, a, nextarg); break; case C_PROXY_CAPATH: /* --proxy-capath */ err = getstr(&config->proxy_capath, nextarg, DENY_BLANK); break; case C_PROXY_CERT: /* --proxy-cert */ - GetFileAndPassword(nextarg, &config->proxy_cert, - &config->proxy_key_passwd); + err = GetFileAndPassword(nextarg, &config->proxy_cert, + &config->proxy_key_passwd); break; case C_PROXY_CRLFILE: /* --proxy-crlfile */ - err = getstr(&config->proxy_crlfile, nextarg, DENY_BLANK); + err = existingfile(&config->proxy_crlfile, a, nextarg); break; case C_PROXY_KEY: /* --proxy-key */ err = getstr(&config->proxy_key, nextarg, ALLOW_BLANK); diff --git a/src/tool_getparam.h b/src/tool_getparam.h index d5ef54a81d..d4812f6ceb 100644 --- a/src/tool_getparam.h +++ b/src/tool_getparam.h @@ -372,9 +372,9 @@ ParameterError getparameter(const char *flag, const char *nextarg, int max_recursive); #ifdef UNITTESTS -void parse_cert_parameter(const char *cert_parameter, - char **certname, - char **passphrase); +ParameterError parse_cert_parameter(const char *cert_parameter, + char **certname, + char **passphrase); #endif ParameterError parse_args(int argc, argv_item_t argv[]); diff --git a/tests/data/test305 b/tests/data/test305 index 2d4ffd1aab..25656d2289 100644 --- a/tests/data/test305 +++ b/tests/data/test305 @@ -19,14 +19,17 @@ https insecure HTTPS without permission -https://%HOSTIP:%HTTPSPORT/want/%TESTNUMBER --cacert moooo --no-ca-native +https://%HOSTIP:%HTTPSPORT/want/%TESTNUMBER --cacert "%LOGDIR/cacert" --no-ca-native + +made up content + # Verify data after the test has been "shot" -77 +77,60 diff --git a/tests/data/test404 b/tests/data/test404 index 8461abfc00..a4fc826ed9 100644 --- a/tests/data/test404 +++ b/tests/data/test404 @@ -19,14 +19,17 @@ ftps FTPS with invalid cacert ---ftp-ssl-control --cacert moooo ftps://%HOSTIP:%FTPSPORT/ +--ftp-ssl-control --cacert "%LOGDIR/cacert" ftps://%HOSTIP:%FTPSPORT/ + +made up content + # Verify data after the test has been "shot" -77 +77,60 diff --git a/tests/data/test697 b/tests/data/test697 index f1958e83e7..d6cfd76136 100644 --- a/tests/data/test697 +++ b/tests/data/test697 @@ -28,7 +28,7 @@ netrc with missing netrc file # Verify data after the test has been "shot" -26 +2 diff --git a/tests/tunit/tool1394.c b/tests/tunit/tool1394.c index 3a5256bae1..71be2c87c4 100644 --- a/tests/tunit/tool1394.c +++ b/tests/tunit/tool1394.c @@ -31,84 +31,91 @@ static CURLcode test_tool1394(const char *arg) { UNITTEST_BEGIN_SIMPLE - static const char *values[] = { + struct cert { + const char *param; + const char *cert; + const char *passwd; + }; + + static const struct cert values[] = { /* -E parameter */ /* exp. cert name */ /* exp. passphrase */ - "foo:bar:baz", "foo", "bar:baz", - "foo\\:bar:baz", "foo:bar", "baz", - "foo\\\\:bar:baz", "foo\\", "bar:baz", - "foo:bar\\:baz", "foo", "bar\\:baz", - "foo:bar\\\\:baz", "foo", "bar\\\\:baz", - "foo\\bar\\baz", "foo\\bar\\baz", NULL, - "foo\\\\bar\\\\baz", "foo\\bar\\baz", NULL, - "foo\\", "foo\\", NULL, - "foo\\\\", "foo\\", NULL, - "foo:bar\\", "foo", "bar\\", - "foo:bar\\\\", "foo", "bar\\\\", - "foo:bar:", "foo", "bar:", - "foo\\::bar\\:", "foo:", "bar\\:", - "pkcs11:foobar", "pkcs11:foobar", NULL, - "PKCS11:foobar", "PKCS11:foobar", NULL, - "PkCs11:foobar", "PkCs11:foobar", NULL, + {"foo:bar:baz", "foo", "bar:baz"}, + {"foo\\:bar:baz", "foo:bar", "baz"}, + {"foo\\\\:bar:baz", "foo\\", "bar:baz"}, + {"foo:bar\\:baz", "foo", "bar\\:baz"}, + {"foo:bar\\\\:baz", "foo", "bar\\\\:baz"}, + {"foo\\bar\\baz", "foo\\bar\\baz", NULL}, + {"foo\\\\bar\\\\baz", "foo\\bar\\baz", NULL}, + {"foo\\", "foo\\", NULL}, + {"foo\\\\", "foo\\", NULL}, + {"foo:bar\\", "foo", "bar\\"}, + {"foo:bar\\\\", "foo", "bar\\\\"}, + {"foo:bar:", "foo", "bar:"}, + {"foo\\::bar\\:", "foo:", "bar\\:"}, + {"pkcs11:foobar", "pkcs11:foobar", NULL}, + {"PKCS11:foobar", "PKCS11:foobar", NULL}, + {"PkCs11:foobar", "PkCs11:foobar", NULL}, #ifdef _WIN32 - "c:\\foo:bar:baz", "c:\\foo", "bar:baz", - "c:\\foo\\:bar:baz", "c:\\foo:bar", "baz", - "c:\\foo\\\\:bar:baz", "c:\\foo\\", "bar:baz", - "c:\\foo:bar\\:baz", "c:\\foo", "bar\\:baz", - "c:\\foo:bar\\\\:baz", "c:\\foo", "bar\\\\:baz", - "c:\\foo\\bar\\baz", "c:\\foo\\bar\\baz", NULL, - "c:\\foo\\\\bar\\\\baz", "c:\\foo\\bar\\baz", NULL, - "c:\\foo\\", "c:\\foo\\", NULL, - "c:\\foo\\\\", "c:\\foo\\", NULL, - "c:\\foo:bar\\", "c:\\foo", "bar\\", - "c:\\foo:bar\\\\", "c:\\foo", "bar\\\\", - "c:\\foo:bar:", "c:\\foo", "bar:", - "c:\\foo\\::bar\\:", "c:\\foo:", "bar\\:", + {"c:\\foo:bar:baz", "c:\\foo", "bar:baz"}, + {"c:\\foo\\:bar:baz", "c:\\foo:bar", "baz"}, + {"c:\\foo\\\\:bar:baz", "c:\\foo\\", "bar:baz"}, + {"c:\\foo:bar\\:baz", "c:\\foo", "bar\\:baz"}, + {"c:\\foo:bar\\\\:baz", "c:\\foo", "bar\\\\:baz"}, + {"c:\\foo\\bar\\baz", "c:\\foo\\bar\\baz", NULL}, + {"c:\\foo\\\\bar\\\\baz", "c:\\foo\\bar\\baz", NULL}, + {"c:\\foo\\", "c:\\foo\\", NULL}, + {"c:\\foo\\\\", "c:\\foo\\", NULL}, + {"c:\\foo:bar\\", "c:\\foo", "bar\\"}, + {"c:\\foo:bar\\\\", "c:\\foo", "bar\\\\"}, + {"c:\\foo:bar:", "c:\\foo", "bar:"}, + {"c:\\foo\\::bar\\:", "c:\\foo:", "bar\\:"}, #endif - NULL, NULL, NULL, + {NULL, NULL, NULL} }; - const char **p; + const struct cert *p; char *certname, *passphrase; - for(p = values; *p; p += 3) { - parse_cert_parameter(p[0], &certname, &passphrase); - if(p[1]) { + ParameterError err; + for(p = &values[0]; p->param; p++) { + err = parse_cert_parameter(p->param, &certname, &passphrase); + if(!err) { if(certname) { - if(strcmp(p[1], certname)) { + if(strcmp(p->cert, certname)) { curl_mprintf("expected certname '%s' but got '%s' " - "for -E param '%s'\n", p[1], certname, p[0]); + "for -E param '%s'\n", p->cert, certname, p->param); fail("assertion failure"); } } else { curl_mprintf("expected certname '%s' but got NULL " - "for -E param '%s'\n", p[1], p[0]); + "for -E param '%s'\n", p->cert, p->param); fail("assertion failure"); } } else { if(certname) { curl_mprintf("expected certname NULL but got '%s' " - "for -E param '%s'\n", certname, p[0]); + "for -E param '%s'\n", certname, p->param); fail("assertion failure"); } } - if(p[2]) { + if(p->passwd) { if(passphrase) { - if(strcmp(p[2], passphrase)) { + if(strcmp(p->passwd, passphrase)) { curl_mprintf("expected passphrase '%s' but got '%s'" - "for -E param '%s'\n", p[2], passphrase, p[0]); + "for -E param '%s'\n", p->passwd, passphrase, p->param); fail("assertion failure"); } } else { curl_mprintf("expected passphrase '%s' but got NULL " - "for -E param '%s'\n", p[2], p[0]); + "for -E param '%s'\n", p->passwd, p->param); fail("assertion failure"); } } else { if(passphrase) { curl_mprintf("expected passphrase NULL but got '%s' " - "for -E param '%s'\n", passphrase, p[0]); + "for -E param '%s'\n", passphrase, p->param); fail("assertion failure"); } }