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 bd90da1ec754b1da9175a171112665b51e5870b4..337ad33280f7c5c6fab06d6b5f73dc0fd225e44c 100644 (file)
--- 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 d5ef54a81d2480445689968bc97bc5b2ba983793..d4812f6cebca879bb18c65beedfe637317267cf7 100644 (file)
--- 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 2d4ffd1aab939fb360c37d9844c3cb34b959577c..25656d2289ec9b9f86f5f8b7883efe75e0abcb72 100644 (file)
--- a/tests/data/test305
+++ b/tests/data/test305
@@ -19,14 +19,17 @@ https
 insecure HTTPS without permission
 </name>
 <command>
-https://%HOSTIP:%HTTPSPORT/want/%TESTNUMBER --cacert moooo --no-ca-native
+https://%HOSTIP:%HTTPSPORT/want/%TESTNUMBER --cacert "%LOGDIR/cacert" --no-ca-native
 </command>
+<file name="%LOGDIR/cacert">
+made up content
+</file>
 </client>
 
 # Verify data after the test has been "shot"
 <verify>
 <errorcode>
-77
+77,60
 </errorcode>
 </verify>
 </testcase>

diff --git a/tests/data/test404 b/tests/data/test404
index 8461abfc004a7337e492a7c8137d7ad24ea3439d..a4fc826ed9f0cf0e744fe12530d50c5af923caf2 100644 (file)
--- a/tests/data/test404
+++ b/tests/data/test404
@@ -19,14 +19,17 @@ ftps
 FTPS with invalid cacert
 </name>
 <command>
---ftp-ssl-control --cacert moooo ftps://%HOSTIP:%FTPSPORT/
+--ftp-ssl-control --cacert "%LOGDIR/cacert" ftps://%HOSTIP:%FTPSPORT/
 </command>
+<file name="%LOGDIR/cacert">
+made up content
+</file>
 </client>
 
 # Verify data after the test has been "shot"
 <verify>
 <errorcode>
-77
+77,60
 </errorcode>
 </verify>
 </testcase>

diff --git a/tests/data/test697 b/tests/data/test697
index f1958e83e7d2e0de0270608ac9910738a87e2419..d6cfd76136280be8cd9f97dd742224182b15aab1 100644 (file)
--- 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"
 <verify>
 <errorcode>
-26
+2
 </errorcode>
 </verify>
 </testcase>

diff --git a/tests/tunit/tool1394.c b/tests/tunit/tool1394.c
index 3a5256bae1fce9cd43ccc7571995be75c2007ae9..71be2c87c478c7ba05b920ca28fb8057b21a6fea 100644 (file)
--- 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");
       }
     }