tool: fix memory mixups

memory allocated by libcurl must be freed with curl_free() and vice versa,
memory allocated by the tool itself must be freed with curlx_free().

- dynbuf: free libcurl data with curl_free()
- tool_operate: make sure we get URL using the right memory
- tool_operhlp: free libcurl memory with curl_free()
- tool_operate: free curl_maprintf() pointer with curl_free
- var: data from curlx_base64_decode needs curlx_free
- tool_operate: fix memory juggling in etag handling
- tool_cb_hdr: fix memory area mixups
- tool_operate: another mixup in etag management
- tool_cb_hdr: more memory mixup fixes
- tool_cfgable.c: document some details
- tool_help: show global-mem-debug in -V output

Closes #21099

diff --git a/lib/curlx/dynbuf.c b/lib/curlx/dynbuf.c
index 88dc436a89ac2d19631954bd3b00772df6fec7ee..a77363ac5927a621a8d440310d71f0ef3a4d9d58 100644 (file)
--- a/lib/curlx/dynbuf.c
+++ b/lib/curlx/dynbuf.c
@@ -205,7 +205,7 @@ CURLcode curlx_dyn_vaddf(struct dynbuf *s, const char *fmt, va_list ap)
 
   if(str) {
     CURLcode result = dyn_nappend(s, (const unsigned char *)str, strlen(str));
-    curlx_free(str);
+    curl_free(str);
     return result;
   }
   /* If we failed, we cleanup the whole buffer and return error */

diff --git a/src/tool_cb_hdr.c b/src/tool_cb_hdr.c
index d10f3e02c1fcc6441f347c0ab8984fe51c3c7f5f..4ee777e1c04807e84a215b280697ba99ee15e4e8 100644 (file)
--- a/src/tool_cb_hdr.c
+++ b/src/tool_cb_hdr.c
@@ -311,12 +311,16 @@ static size_t content_disposition(const char *str, const char *end,
           return CURL_WRITEFUNC_ERROR;
         }
         if(outs->alloc_filename)
-          curlx_free(outs->filename);
+          tool_safefree(outs->filename);
 
         if(per->config->output_dir) {
-          outs->filename = curl_maprintf("%s/%s", per->config->output_dir,
-                                         filename);
+          char *f = curl_maprintf("%s/%s", per->config->output_dir,
+                                  filename);
           curlx_free(filename);
+          if(!f)
+            return CURL_WRITEFUNC_ERROR;
+          outs->filename = curlx_strdup(f);
+          curl_free(f);
           if(!outs->filename)
             return CURL_WRITEFUNC_ERROR;
         }
@@ -362,12 +366,16 @@ static size_t content_disposition(const char *str, const char *end,
           return CURL_WRITEFUNC_ERROR;
         }
         if(outs->alloc_filename)
-          curlx_free(outs->filename);
+          tool_safefree(outs->filename);
 
         if(per->config->output_dir) {
-          outs->filename = curl_maprintf("%s/%s", per->config->output_dir,
-                                         filename);
+          char *f = curl_maprintf("%s/%s", per->config->output_dir,
+                                  filename);
           curlx_free(filename);
+          if(!f)
+            return CURL_WRITEFUNC_ERROR;
+          outs->filename = curlx_strdup(f);
+          curl_free(f);
           if(!outs->filename)
             return CURL_WRITEFUNC_ERROR;
         }
@@ -396,7 +404,7 @@ static size_t content_disposition(const char *str, const char *end,
      hdrcbdata->config->show_headers) {
     /* still awaiting the Content-Disposition header, store the header in
        memory. Since it is not null-terminated, we need an extra dance. */
-    char *clone = curl_maprintf("%.*s", (int)cb, str);
+    char *clone = curlx_memdup0(str, cb);
     if(clone) {
       struct curl_slist *old = hdrcbdata->headlist;
       hdrcbdata->headlist = curl_slist_append(old, clone);

diff --git a/src/tool_cfgable.c b/src/tool_cfgable.c
index 381ca874d9daa7fef4f981cc437a67875f7e3ba6..0bfeac6f1fa384f952738fdb6df9c515b9b8501c 100644 (file)
--- a/src/tool_cfgable.c
+++ b/src/tool_cfgable.c
@@ -219,8 +219,17 @@ void config_free(struct OperationConfig *config)
  *
  * The main point is to make sure that what is returned is different than what
  * the regular memory functions return so that mixup will trigger problems.
+ *
+ * This test setup currently only works when building with a *shared* libcurl
+ * and not static, as in the latter case the tool and the library share some of
+ * the functions in incompatible ways.
  */
 
+/*
+ * This code appends this extra chunk of memory in front of every allocation
+ * done by libcurl with the only purpose to cause trouble when using the wrong
+ * free function on memory.
+ */
 struct extramem {
   size_t extra;
   union {
@@ -254,18 +263,17 @@ static void *custom_malloc(size_t wanted_size)
 static char *custom_strdup(const char *ptr)
 {
   struct extramem *m;
-  size_t size = strlen(ptr) + 1;
-  size_t sz = size + sizeof(struct extramem);
+  size_t len = strlen(ptr);
+  size_t sz = len + sizeof(struct extramem);
   m = curlx_malloc(sz);
   if(m) {
     char *p = (char *)m->mem;
     /* since strcpy is banned, we do memcpy */
-    memcpy(p, ptr, sz);
-    p[sz] = 0;
+    memcpy(p, ptr, len);
+    p[len] = 0;
     return (char *)m->mem;
   }
   return NULL;
-
 }
 
 static void *custom_realloc(void *ptr, size_t size)
@@ -273,7 +281,8 @@ static void *custom_realloc(void *ptr, size_t size)
   struct extramem *m = NULL;
   size_t sz = size + sizeof(struct extramem);
   if(ptr)
-    m = (void *)((char *)ptr - offsetof(struct extramem, mem));
+    /* if given a pointer, figure out the original */
+    ptr = (void *)((char *)ptr - offsetof(struct extramem, mem));
   m = curlx_realloc(ptr, sz);
   if(m)
     return m->mem;
@@ -315,7 +324,7 @@ CURLcode globalconf_init(void)
   global->first = global->last = config_alloc();
   if(global->first) {
     /* Perform the libcurl initialization */
-#ifdef GLOBAL_MEM
+#ifdef CURL_DEBUG_GLOBAL_MEM
     result = curl_global_init_mem(CURL_GLOBAL_ALL, custom_malloc, custom_free,
                                   custom_realloc, custom_strdup,
                                   custom_calloc);

diff --git a/src/tool_help.c b/src/tool_help.c
index 950fad16fabdb10cd4f865842d397d4611432d86..212972f382480a5a65f6cb44427da24c5d7b02a2 100644 (file)
--- a/src/tool_help.c
+++ b/src/tool_help.c
@@ -356,6 +356,9 @@ void tool_version_info(void)
     size_t feat_ext_count = feature_count;
 #ifdef CURL_CA_EMBED
     ++feat_ext_count;
+#endif
+#ifdef CURL_DEBUG_GLOBAL_MEM
+    ++feat_ext_count;
 #endif
     feat_ext = curlx_malloc(sizeof(*feature_names) * (feat_ext_count + 1));
     if(feat_ext) {
@@ -364,6 +367,9 @@ void tool_version_info(void)
       feat_ext_count = feature_count;
 #ifdef CURL_CA_EMBED
       feat_ext[feat_ext_count++] = "CAcert";
+#endif
+#ifdef CURL_DEBUG_GLOBAL_MEM
+      feat_ext[feat_ext_count++] = "global-mem-debug";
 #endif
       feat_ext[feat_ext_count] = NULL;
       qsort((void *)feat_ext, feat_ext_count, sizeof(*feat_ext),

diff --git a/src/tool_operate.c b/src/tool_operate.c
index fe996c6174eca985085b8d30e8896330d35d3e31..9348d78a909f7dc880dca8b677f6debf53e776d3 100644 (file)
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -778,7 +778,7 @@ static CURLcode post_per_transfer(struct per_transfer *per,
     tool_safefree(per->etag_save.filename);
 
   if(outs->alloc_filename)
-    curlx_free(outs->filename);
+    tool_safefree(outs->filename);
   curl_slist_free_all(per->hdrcbdata.headlist);
   per->hdrcbdata.headlist = NULL;
   return result;
@@ -848,8 +848,13 @@ static CURLcode append2query(struct OperationConfig *config,
       if(uerr)
         result = urlerr_cvt(uerr);
       else {
+        /* use our new URL instead! */
         curlx_free(per->url); /* free previous URL */
-        per->url = updated; /* use our new URL instead! */
+        /* make it allocated by tool memory */
+        per->url = curlx_strdup(updated);
+        curl_free(updated);
+        if(!per->url)
+          result = CURLE_OUT_OF_MEMORY;
       }
     }
     curl_url_cleanup(uh);
@@ -876,11 +881,16 @@ static CURLcode etag_compare(struct OperationConfig *config)
 
   if((PARAM_OK == file2string(&etag_from_file, file)) &&
      etag_from_file) {
-    header = curl_maprintf("If-None-Match: %s", etag_from_file);
+    char *h = curl_maprintf("If-None-Match: %s", etag_from_file);
     tool_safefree(etag_from_file);
+    if(h) {
+      /* move it to the right memory */
+      header = curlx_strdup(h);
+      curl_free(h);
+    }
   }
   else
-    header = curl_maprintf("If-None-Match: \"\"");
+    header = curlx_strdup("If-None-Match: \"\"");
 
   if(!header) {
     if(file)
@@ -891,7 +901,7 @@ static CURLcode etag_compare(struct OperationConfig *config)
 
   /* add Etag from file to list of custom headers */
   pe = add2list(&config->headers, header);
-  tool_safefree(header);
+  curlx_free(header);
 
   if(file)
     curlx_fclose(file);
@@ -1042,10 +1052,13 @@ static CURLcode setup_outfile(struct OperationConfig *config,
 
   if(config->output_dir && *config->output_dir) {
     char *d = curl_maprintf("%s/%s", config->output_dir, per->outfile);
-    if(!d)
+    tool_safefree(per->outfile);
+    if(d) {
+      per->outfile = curlx_strdup(d); /* move to right memory */
+      curl_free(d);
+    }
+    if(!d || !per->outfile)
       return CURLE_WRITE_ERROR;
-    curlx_free(per->outfile);
-    per->outfile = d;
   }
   /* Create the directory hierarchy, if not pre-existent to a multiple
      file output call */

diff --git a/src/tool_operhlp.c b/src/tool_operhlp.c
index 5c50597393235943ab2ff239e2e1917df4a9c29f..a0ae58f2235aaabfab65358c3fca0d7f4fe4fc74 100644 (file)
--- a/src/tool_operhlp.c
+++ b/src/tool_operhlp.c
@@ -146,7 +146,7 @@ CURLcode add_file_name_to_url(CURL *curl, char **inurlp, const char *filename)
         if(!newpath)
           goto out;
         uerr = curl_url_set(uh, CURLUPART_PATH, newpath, 0);
-        curlx_free(newpath);
+        curl_free(newpath);
         if(uerr) {
           result = urlerr_cvt(uerr);
           goto out;
@@ -157,8 +157,13 @@ CURLcode add_file_name_to_url(CURL *curl, char **inurlp, const char *filename)
           goto out;
         }
         curlx_free(*inurlp);
-        *inurlp = newurl;
-        result = CURLE_OK;
+        /* make it owned by tool memory */
+        *inurlp = curlx_strdup(newurl);
+        curl_free(newurl);
+        if(*inurlp)
+          result = CURLE_OK;
+        else
+          result = CURLE_OUT_OF_MEMORY;
       }
     }
     else

diff --git a/src/var.c b/src/var.c
index 9065e0c0a93cb388a7214fdad1ce1b0fe3417b5a..a149751d9f0f35cbde4e6b0a54a0b04f841b0087 100644 (file)
--- a/src/var.c
+++ b/src/var.c
@@ -153,7 +153,7 @@ static ParameterError varfunc(char *c, /* content */
         /* put it in the output */
         if(curlx_dyn_addn(out, enc, elen))
           err = PARAM_NO_MEM;
-        curl_free(enc);
+        curlx_free(enc);
         if(err)
           break;
       }
@@ -173,7 +173,7 @@ static ParameterError varfunc(char *c, /* content */
         else {
           if(curlx_dyn_addn(out, enc, elen))
             err = PARAM_NO_MEM;
-          curl_free(enc);
+          curlx_free(enc);
         }
         if(err)
           break;