Hi,
After checking out the code per instructions on http://curl.haxx.se/cvs.html,
I reviewed parts of curl sources and spotted some infelicities:

  - NULL-deref after failed realloc in lib/base64.c (suck)

  - that same function uses "int" as buffer size, and doubles/reallocs,
    but doesn't check for overflow, so if you give it 2GB of data, it'll
    wrap and do "bad things"

==============================================================
I do see that the suck function is conditionally compiled,
and is used only for testing, but still.

  - in lib/formdata.c (Curl_getFormData) a failed strdup (in use of strippath)
    yields a NULL filebasename, which is dereferenced in AddFormDataf when
    !post->showfilename

==============================================================
In main.c, in the MSDOS-only block below,
don't fall through if strdup returns NULL.
Otherwise, create_dir_hierarchy(NULL,... will call
strdup(NULL) and you lose.

  #if defined(MSDOS)
              {
                /* This is for DOS, and then we do some major replacing of
                   bad characters in the file name before using it */
                char file1 [PATH_MAX];

                strcpy(file1, msdosify(outfile));
                free (outfile);
                outfile = strdup (rename_if_dos_device_name(file1));
              }
  #endif /* MSDOS */
            }
            else if(urls) {
              /* fill '#1' ... '#9' terms from URL pattern */
              char *storefile = outfile;
              outfile = glob_match_url(storefile, urls);
              free(storefile);
              if(!outfile) {
                /* bad globbing */
                warnf(config, "bad output glob!\n");
                free(url);
                res = CURLE_FAILED_INIT;
                break;
              }
            }

            /* Create the directory hierarchy, if not pre-existant to a multiple
               file output call */

            if(config->create_dirs &&
               (-1 == create_dir_hierarchy(outfile, config->errors)))
              return CURLE_WRITE_ERROR;

==============================================================
Also in main.c, in this function, you'll want to handle failure
of strdup(buf). Otherwise, strchr(NULL,... will cause a NULL dereference:

static char *my_get_line(FILE *fp)
{
   char buf[4096];
   char *nl = NULL;
   char *retval = NULL;

   do {
     if (NULL == fgets(buf, sizeof(buf), fp))
       break;
     if (NULL == retval)
       retval = strdup(buf);
     else {
       if (NULL == (retval = realloc(retval,
                                     strlen(retval) + strlen(buf) + 1)))
         break;
       strcat(retval, buf);
     }
   }
   while (NULL == (nl = strchr(retval, '\n')));

==============================================================
in ares/ares_parse_ptr_reply.c, this realloc return value is not
checked, and may later be dereferenced:

        aliases = realloc(aliases, (aliascnt/16+1) * sizeof(char *));
        ...
                      for (i=0 ; i<aliascnt ; i++)
                        hostent->h_aliases[i] = aliases[i];

==============================================================
apparently-unchecked malloc calls:
(but there are so many, maybe malloc is a wrapper?)

./lib/nss.c: slotname = malloc(SLOTSIZE);
./lib/nss.c: nickname = malloc(PATH_MAX);
./lib/nss.c- snprintf(slotname, SLOTSIZE, "PEM Token #%ld", slotID);
./lib/nss.c- snprintf(nickname, PATH_MAX, "PEM Token #%ld:%s", slotID, n);

./lib/nss.c: slotname = malloc(SLOTSIZE);
./lib/nss.c- snprintf(slotname, SLOTSIZE, "PEM Token #%ld", slotID);

./lib/nss.c: parg = malloc(sizeof(pphrase_arg_t));
./lib/nss.c- parg->retryCount = 0;

...
========================================
in lib/krb4.c, a failed malloc leads to NULL-deref, because
krb_mk_safe appears not to deal gracefully with a NULL pointer.

static int
krb4_encode(void *app_data, const void *from, int length, int level, void **to,
            struct connectdata *conn)
{
  struct krb4_data *d = app_data;
  *to = malloc(length + 31);
  if(level == prot_safe)
    /* NOTE that the void* cast is safe, krb_mk_safe/priv don't modify the
     * input buffer
     */
    return krb_mk_safe((void*)from, *to, length, &d->key,
                       (struct sockaddr_in *)LOCAL_ADDR,
                       (struct sockaddr_in *)REMOTE_ADDR);

========================================

I stopped there, after having examined only about 12% of the malloc uses.
I hope someone can find the time to complete the job.
Received on 2008-11-13