tool_urlglob: clean up used memory on errors better

Previously it had to realloc the pattern array to store the last entry
even when that last entry triggered an error and could be only half
filled in.

Also cleaned up for readability and better reallocs for sets.

Reported-by: letshack9707 on hackerone
Closes #19614

diff --git a/src/tool_urlglob.c b/src/tool_urlglob.c
index 4a28376e144453467e38b1d1854226631c081484..7185e80f66b1db63b0e04d180f6ebd560205c38f 100644 (file)
--- a/src/tool_urlglob.c
+++ b/src/tool_urlglob.c
@@ -40,21 +40,24 @@ static CURLcode globerror(struct URLGlob *glob, const char *err,
 
 static CURLcode glob_fixed(struct URLGlob *glob, char *fixed, size_t len)
 {
-  struct URLPattern *pat = &glob->pattern[glob->size];
+  struct URLPattern *pat = &glob->pattern[glob->pnum];
   pat->type = GLOB_SET;
-  pat->c.set.size = 1;
-  pat->c.set.idx = 0;
   pat->globindex = -1;
-
+  pat->c.set.size = 0;
+  pat->c.set.idx = 0;
   pat->c.set.elem = malloc(sizeof(char *));
 
   if(!pat->c.set.elem)
     return globerror(glob, NULL, 0, CURLE_OUT_OF_MEMORY);
 
   pat->c.set.elem[0] = memdup0(fixed, len);
-  if(!pat->c.set.elem[0])
+  if(!pat->c.set.elem[0]) {
+    tool_safefree(pat->c.set.elem);
     return globerror(glob, NULL, 0, CURLE_OUT_OF_MEMORY);
+  }
 
+  pat->c.set.palloc = 1;
+  pat->c.set.size = 1;
   return CURLE_OK;
 }
 
@@ -98,59 +101,66 @@ static CURLcode glob_set(struct URLGlob *glob, const char **patternp,
   const char *pattern = *patternp;
   const char *opattern = pattern;
   size_t opos = *posp-1;
-
-  pat = &glob->pattern[glob->size];
-  /* patterns 0,1,2,... correspond to size=1,3,5,... */
-  pat->type = GLOB_SET;
-  pat->c.set.size = 0;
-  pat->c.set.idx = 0;
-  pat->c.set.elem = NULL;
-  pat->globindex = globindex;
+  CURLcode result = CURLE_OK;
+  size_t size = 0;
+  char **elem = NULL;
+  size_t palloc = 0; /* start with this */
 
   while(!done) {
     switch(*pattern) {
     case '\0':                  /* URL ended while set was still open */
-      return globerror(glob, "unmatched brace", opos, CURLE_URL_MALFORMAT);
+      result = globerror(glob, "unmatched brace", opos, CURLE_URL_MALFORMAT);
+      goto error;
 
     case '{':
     case '[':                   /* no nested expressions at this time */
-      return globerror(glob, "nested brace", *posp, CURLE_URL_MALFORMAT);
+      result = globerror(glob, "nested brace", *posp, CURLE_URL_MALFORMAT);
+      goto error;
 
     case '}':                           /* set element completed */
-      if(opattern == pattern)
-        return globerror(glob, "empty string within braces", *posp,
-                         CURLE_URL_MALFORMAT);
+      if(opattern == pattern) {
+        result = globerror(glob, "empty string within braces", *posp,
+                           CURLE_URL_MALFORMAT);
+        goto error;
+      }
 
       /* add 1 to size since it will be incremented below */
-      if(multiply(amount, pat->c.set.size + 1))
-        return globerror(glob, "range overflow", 0, CURLE_URL_MALFORMAT);
+      if(multiply(amount, size + 1)) {
+        result = globerror(glob, "range overflow", 0, CURLE_URL_MALFORMAT);
+        goto error;
+      }
       done = TRUE;
       FALLTHROUGH();
     case ',':
-      if(pat->c.set.elem) {
-        char **arr;
-
-        if(pat->c.set.size >= (curl_off_t)(SIZE_MAX/(sizeof(char *))))
-          return globerror(glob, "range overflow", 0, CURLE_URL_MALFORMAT);
-
-        arr = realloc(pat->c.set.elem, (size_t)(pat->c.set.size + 1) *
-                      sizeof(char *));
-        if(!arr)
-          return globerror(glob, NULL, 0, CURLE_OUT_OF_MEMORY);
+      if(size >= 100000)
+        return globerror(glob, "range overflow", 0, CURLE_URL_MALFORMAT);
 
-        pat->c.set.elem = arr;
+      if(!palloc) {
+        palloc = 5; /* a reasonable default */
+        elem = malloc(palloc * sizeof(char *));
+      }
+      else if(size >= palloc) {
+        char **arr = realloc(elem, palloc * 2 * sizeof(char *));
+        if(!arr) {
+          result = globerror(glob, NULL, 0, CURLE_OUT_OF_MEMORY);
+          goto error;
+        }
+        elem = arr;
+        palloc *= 2;
       }
-      else
-        pat->c.set.elem = malloc(sizeof(char *));
 
-      if(!pat->c.set.elem)
-        return globerror(glob, NULL, 0, CURLE_OUT_OF_MEMORY);
+      if(!elem) {
+        result = globerror(glob, NULL, 0, CURLE_OUT_OF_MEMORY);
+        goto error;
+      }
 
-      pat->c.set.elem[pat->c.set.size] = strdup(curlx_dyn_ptr(&glob->buf) ?
-                                                curlx_dyn_ptr(&glob->buf): "");
-      if(!pat->c.set.elem[pat->c.set.size])
-        return globerror(glob, NULL, 0, CURLE_OUT_OF_MEMORY);
-      ++pat->c.set.size;
+      elem[size] =
+        strdup(curlx_dyn_ptr(&glob->buf) ? curlx_dyn_ptr(&glob->buf) : "");
+      if(!elem[size]) {
+        result = globerror(glob, NULL, 0, CURLE_OUT_OF_MEMORY);
+        goto error;
+      }
+      ++size;
       curlx_dyn_reset(&glob->buf);
 
       ++pattern;
@@ -159,8 +169,9 @@ static CURLcode glob_set(struct URLGlob *glob, const char **patternp,
       break;
 
     case ']':                           /* illegal closing bracket */
-      return globerror(glob, "unexpected close bracket", *posp,
-                       CURLE_URL_MALFORMAT);
+      result = globerror(glob, "unexpected close bracket", *posp,
+                         CURLE_URL_MALFORMAT);
+      goto error;
 
     case '\\':                          /* escaped character, skip '\' */
       if(pattern[1]) {
@@ -170,14 +181,33 @@ static CURLcode glob_set(struct URLGlob *glob, const char **patternp,
       FALLTHROUGH();
     default:
       /* copy character to set element */
-      if(curlx_dyn_addn(&glob->buf, pattern++, 1))
-        return CURLE_OUT_OF_MEMORY;
+      if(curlx_dyn_addn(&glob->buf, pattern++, 1)) {
+        result = CURLE_OUT_OF_MEMORY;
+        goto error;
+      }
       ++(*posp);
     }
   }
 
   *patternp = pattern; /* return with the new position */
+
+  pat = &glob->pattern[glob->pnum];
+  pat->type = GLOB_SET;
+  pat->globindex = globindex;
+  pat->c.set.elem = elem;
+  pat->c.set.size = size;
+  pat->c.set.idx = 0;
+  pat->c.set.palloc = palloc;
+
   return CURLE_OK;
+error:
+  {
+    size_t i;
+    for(i = 0; i < size; i++)
+      tool_safefree(elem[i]);
+  }
+  free(elem);
+  return result;
 }
 
 static CURLcode glob_range(struct URLGlob *glob, const char **patternp,
@@ -194,7 +224,7 @@ static CURLcode glob_range(struct URLGlob *glob, const char **patternp,
   const char *pattern = *patternp;
   const char *c;
 
-  pat = &glob->pattern[glob->size];
+  pat = &glob->pattern[glob->pnum];
   pat->globindex = globindex;
 
   if(ISALPHA(*pattern)) {
@@ -349,6 +379,25 @@ static bool peek_ipv6(const char *str, size_t *skip)
   return rc ? FALSE : TRUE;
 }
 
+static CURLcode add_glob(struct URLGlob *glob, size_t pos)
+{
+  DEBUGASSERT(glob->pattern[glob->pnum].type);
+
+  if(++glob->pnum >= glob->palloc) {
+    struct URLPattern *np = NULL;
+    glob->palloc *= 2;
+    if(glob->pnum < 255) { /* avoid ridiculous amounts */
+      np = realloc(glob->pattern, glob->palloc * sizeof(struct URLPattern));
+      if(!np)
+        return globerror(glob, NULL, pos, CURLE_OUT_OF_MEMORY);
+    }
+    else
+      return globerror(glob, "too many {} sets", pos, CURLE_URL_MALFORMAT);
+    glob->pattern = np;
+  }
+  return CURLE_OK;
+}
+
 static CURLcode glob_parse(struct URLGlob *glob, const char *pattern,
                            size_t pos, curl_off_t *amount)
 {
@@ -381,8 +430,8 @@ static CURLcode glob_parse(struct URLGlob *glob, const char *pattern,
 
       /* only allow \ to escape known "special letters" */
       if(*pattern == '\\' &&
-         (*(pattern + 1) == '{' || *(pattern + 1) == '[' ||
-          *(pattern + 1) == '}' || *(pattern + 1) == ']') ) {
+         (pattern[1] == '{' || pattern[1] == '[' ||
+          pattern[1] == '}' || pattern[1] == ']') ) {
 
         /* escape character, skip '\' */
         ++pattern;
@@ -397,41 +446,30 @@ static CURLcode glob_parse(struct URLGlob *glob, const char *pattern,
       /* we got a literal string, add it as a single-item list */
       res = glob_fixed(glob, curlx_dyn_ptr(&glob->buf),
                        curlx_dyn_len(&glob->buf));
+      if(!res)
+        res = add_glob(glob, pos);
       curlx_dyn_reset(&glob->buf);
     }
     else {
-      switch(*pattern) {
-      case '\0': /* done  */
+      if(!*pattern) /* done  */
         break;
-
-      case '{':
+      else if(*pattern =='{') {
         /* process set pattern */
         pattern++;
         pos++;
         res = glob_set(glob, &pattern, &pos, amount, globindex++);
-        break;
-
-      case '[':
+        if(!res)
+          res = add_glob(glob, pos);
+      }
+      else if(*pattern == '[') {
         /* process range pattern */
         pattern++;
         pos++;
         res = glob_range(glob, &pattern, &pos, amount, globindex++);
-        break;
+        if(!res)
+          res = add_glob(glob, pos);
       }
     }
-
-    if(++glob->size >= glob->palloc) {
-      struct URLPattern *np = NULL;
-      glob->palloc *= 2;
-      if(glob->size < 255) { /* avoid ridiculous amounts */
-        np = realloc(glob->pattern, glob->palloc * sizeof(struct URLPattern));
-        if(!np)
-          return globerror(glob, NULL, pos, CURLE_OUT_OF_MEMORY);
-      }
-      else
-        return globerror(glob, "too many {} sets", pos, CURLE_URL_MALFORMAT);
-      glob->pattern = np;
-    }
   }
   return res;
 }
@@ -459,9 +497,7 @@ CURLcode glob_url(struct URLGlob *glob, char *url, curl_off_t *urlnum,
   glob->palloc = 2;
 
   res = glob_parse(glob, url, 1, &amount);
-  if(!res)
-    *urlnum = amount;
-  else {
+  if(res) {
     if(error && glob->error) {
       char text[512];
       const char *t;
@@ -477,25 +513,24 @@ CURLcode glob_url(struct URLGlob *glob, char *url, curl_off_t *urlnum,
       /* send error description to the error-stream */
       curl_mfprintf(error, "curl: (%d) %s\n", res, t);
     }
-    /* it failed, we cleanup */
-    glob_cleanup(glob);
     *urlnum = 1;
     return res;
   }
-
+  *urlnum = amount;
   return CURLE_OK;
 }
 
 void glob_cleanup(struct URLGlob *glob)
 {
   size_t i;
-  curl_off_t elem;
 
   if(glob->pattern) {
-    for(i = 0; i < glob->size; i++) {
+    for(i = 0; i < glob->pnum; i++) {
+      DEBUGASSERT(glob->pattern[i].type);
       if((glob->pattern[i].type == GLOB_SET) &&
          (glob->pattern[i].c.set.elem)) {
-        for(elem = glob->pattern[i].c.set.size - 1; elem >= 0; --elem)
+        curl_off_t elem;
+        for(elem = 0; elem < glob->pattern[i].c.set.size; elem++)
           tool_safefree(glob->pattern[i].c.set.elem[elem]);
         tool_safefree(glob->pattern[i].c.set.elem);
       }
@@ -521,9 +556,9 @@ CURLcode glob_next_url(char **globbed, struct URLGlob *glob)
 
     /* implement a counter over the index ranges of all patterns, starting
        with the rightmost pattern */
-    for(i = 0; carry && (i < glob->size); i++) {
+    for(i = 0; carry && (i < glob->pnum); i++) {
       carry = FALSE;
-      pat = &glob->pattern[glob->size - 1 - i];
+      pat = &glob->pattern[glob->pnum - 1 - i];
       switch(pat->type) {
       case GLOB_SET:
         if((pat->c.set.elem) && (++pat->c.set.idx == pat->c.set.size)) {
@@ -555,7 +590,7 @@ CURLcode glob_next_url(char **globbed, struct URLGlob *glob)
     }
   }
 
-  for(i = 0; i < glob->size; ++i) {
+  for(i = 0; i < glob->pnum; ++i) {
     pat = &glob->pattern[i];
     switch(pat->type) {
     case GLOB_SET:
@@ -581,7 +616,8 @@ CURLcode glob_next_url(char **globbed, struct URLGlob *glob)
     }
   }
 
-  *globbed = strdup(curlx_dyn_ptr(&glob->buf));
+  *globbed =
+    strdup(curlx_dyn_ptr(&glob->buf) ? curlx_dyn_ptr(&glob->buf) : "");
   if(!*globbed)
     return CURLE_OUT_OF_MEMORY;
 
@@ -605,11 +641,11 @@ CURLcode glob_match_url(char **output, const char *filename,
       curl_off_t num;
       struct URLPattern *pat = NULL;
       filename++;
-      if(!curlx_str_number(&filename, &num, glob->size) && num) {
+      if(!curlx_str_number(&filename, &num, glob->pnum) && num) {
         size_t i;
         num--; /* make it zero based */
         /* find the correct glob entry */
-        for(i = 0; i < glob->size; i++) {
+        for(i = 0; i < glob->pnum; i++) {
           if(glob->pattern[i].globindex == (int)num) {
             pat = &glob->pattern[i];
             break;

diff --git a/src/tool_urlglob.h b/src/tool_urlglob.h
index 4bcf229ecceae32edbbd5f533094abe7d17fe3fb..d0a807f7960063de3401cce0b29134e7b98f59b1 100644 (file)
--- a/src/tool_urlglob.h
+++ b/src/tool_urlglob.h
@@ -40,6 +40,7 @@ struct URLPattern {
       char **elem;
       curl_off_t size;
       curl_off_t idx;
+      size_t palloc; /* elem entries allocated */
     } set;
     struct {
       int min;
@@ -64,7 +65,7 @@ struct URLGlob {
   struct dynbuf buf;
   struct URLPattern *pattern;
   size_t palloc; /* number of pattern entries allocated */
-  size_t size;
+  size_t pnum; /* number of patterns used */
   char beenhere;
   const char *error; /* error message */
   size_t pos;        /* column position of error or 0 */