checksrc: ban unsafe functions

The list of unsafe functions currently consists of sprintf, vsprintf,
strcat, strncat and gets.

Subsequently, some existing code needed updating to avoid warnings on
this.

include/curl/mprintf.h

@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2006, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2013, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -58,7 +58,7 @@ CURL_EXTERN char *curl_mvaprintf(const char *format, va_list args);
 # define printf curl_mprintf
 # define fprintf curl_mfprintf
 #ifdef CURLDEBUG
-/* When built with CURLDEBUG we define away the sprintf() functions since we
+/* When built with CURLDEBUG we define away the sprintf functions since we
    don't want internal code to be using them */
 # define sprintf sprintf_was_used
 # define vsprintf vsprintf_was_used

lib/checksrc.pl

@@ -6,7 +6,7 @@
 #                            | (__| |_| |  _ <| |___
 #                             \___|\___/|_| \_\_____|
 #
-# Copyright (C) 2011, Daniel Stenberg, <daniel@haxx.se>, et al.
+# Copyright (C) 2011 - 2013, Daniel Stenberg, <daniel@haxx.se>, et al.
 #
 # This software is licensed as described in the file COPYING, which
 # you should have received as part of this distribution. The terms
@@ -153,6 +153,12 @@ sub scanfile {
             checkwarn($line, length($1)+1, $file, $l, "missing space after close paren");
         }
 
+        # scan for use of banned functions
+        if($l =~ /^(.*\W)(sprintf|vsprintf|strcat|strncat|gets)\s*\(/) {
+            checkwarn($line, length($1), $file, $l,
+                      "use of $2 is banned");
+        }
+
         # check for open brace first on line but not first column
         # only alert if previous line ended with a close paren and wasn't a cpp
         # line

lib/ftp.c

@@ -3978,16 +3978,11 @@ static CURLcode wc_statemach(struct connectdata *conn)
     /* filelist has at least one file, lets get first one */
     struct ftp_conn *ftpc = &conn->proto.ftpc;
     struct curl_fileinfo *finfo = wildcard->filelist->head->ptr;
-    char *tmp_path = malloc(strlen(conn->data->state.path) +
-                      strlen(finfo->filename) + 1);
-    if(!tmp_path) {
-      return CURLE_OUT_OF_MEMORY;
-    }
 
-    tmp_path[0] = 0;
+    char *tmp_path = aprintf("%s%s", wildcard->path, finfo->filename);
-    /* make full path to matched file */
+    if(!tmp_path)
-    strcat(tmp_path, wildcard->path);
+      return CURLE_OUT_OF_MEMORY;
-    strcat(tmp_path, finfo->filename);
+
     /* switch default "state.pathbuffer" and tmp_path, good to see
        ftp_parse_url_path function to understand this trick */
     Curl_safefree(conn->data->state.pathbuffer);
@@ -4124,13 +4119,13 @@ CURLcode Curl_ftpsendf(struct connectdata *conn,
 
   va_list ap;
   va_start(ap, fmt);
-  vsnprintf(s, SBUF_SIZE-3, fmt, ap);
+  write_len = vsnprintf(s, SBUF_SIZE-3, fmt, ap);
   va_end(ap);
 
-  strcat(s, "\r\n"); /* append a trailing CRLF */
+  strcpy(&s[write_len], "\r\n"); /* append a trailing CRLF */
+  write_len +=2;
 
   bytes_written=0;
-  write_len = strlen(s);
 
   res = Curl_convert_to_network(conn->data, s, write_len);
   /* Curl_convert_to_network calls failf if unsuccessful */

lib/http_digest.c

@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2013, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -287,6 +287,7 @@ CURLcode Curl_output_digest(struct connectdata *conn,
   struct timeval now;
 
   char **allocuserpwd;
+  size_t userlen;
   const char *userp;
   const char *passwdp;
   struct auth *authp;
@@ -533,10 +534,11 @@ CURLcode Curl_output_digest(struct connectdata *conn,
   }
 
   /* append CRLF + zero (3 bytes) to the userpwd header */
-  tmp = realloc(*allocuserpwd, strlen(*allocuserpwd) + 3);
+  userlen = strlen(*allocuserpwd);
+  tmp = realloc(*allocuserpwd, userlen + 3);
   if(!tmp)
     return CURLE_OUT_OF_MEMORY;
-  strcat(tmp, "\r\n");
+  strcpy(&tmp[userlen], "\r\n"); /* append the data */
   *allocuserpwd = tmp;
 
   return CURLE_OK;

lib/mprintf.c

@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1999 - 2011, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1999 - 2013, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -203,101 +203,6 @@ static int dprintf_IsQualifierNoDollar(char c)
   }
 }
 
-#ifdef DPRINTF_DEBUG2
-static void dprintf_Pass1Report(va_stack_t *vto, int max)
-{
-  int i;
-  char buffer[256];
-  int bit;
-  int flags;
-
-  for(i=0; i<max; i++) {
-    char *type;
-    switch(vto[i].type) {
-    case FORMAT_UNKNOWN:
-      type = "unknown";
-      break;
-    case FORMAT_STRING:
-      type ="string";
-      break;
-    case FORMAT_PTR:
-      type ="pointer";
-      break;
-    case FORMAT_INT:
-      type = "int";
-      break;
-    case FORMAT_INTPTR:
-      type = "intptr";
-      break;
-    case FORMAT_LONG:
-      type = "long";
-      break;
-    case FORMAT_LONGLONG:
-      type = "long long";
-      break;
-    case FORMAT_DOUBLE:
-      type = "double";
-      break;
-    case FORMAT_LONGDOUBLE:
-      type = "long double";
-      break;
-    }
-
-
-    buffer[0]=0;
-
-    for(bit=0; bit<31; bit++) {
-      flags = vto[i].flags & (1<<bit);
-
-      if(flags & FLAGS_SPACE)
-        strcat(buffer, "space ");
-      else if(flags & FLAGS_SHOWSIGN)
-        strcat(buffer, "plus ");
-      else if(flags & FLAGS_LEFT)
-        strcat(buffer, "left ");
-      else if(flags & FLAGS_ALT)
-        strcat(buffer, "alt ");
-      else if(flags & FLAGS_SHORT)
-        strcat(buffer, "short ");
-      else if(flags & FLAGS_LONG)
-        strcat(buffer, "long ");
-      else if(flags & FLAGS_LONGLONG)
-        strcat(buffer, "longlong ");
-      else if(flags & FLAGS_LONGDOUBLE)
-        strcat(buffer, "longdouble ");
-      else if(flags & FLAGS_PAD_NIL)
-        strcat(buffer, "padnil ");
-      else if(flags & FLAGS_UNSIGNED)
-        strcat(buffer, "unsigned ");
-      else if(flags & FLAGS_OCTAL)
-        strcat(buffer, "octal ");
-      else if(flags & FLAGS_HEX)
-        strcat(buffer, "hex ");
-      else if(flags & FLAGS_UPPER)
-        strcat(buffer, "upper ");
-      else if(flags & FLAGS_WIDTH)
-        strcat(buffer, "width ");
-      else if(flags & FLAGS_WIDTHPARAM)
-        strcat(buffer, "widthparam ");
-      else if(flags & FLAGS_PREC)
-        strcat(buffer, "precision ");
-      else if(flags & FLAGS_PRECPARAM)
-        strcat(buffer, "precparam ");
-      else if(flags & FLAGS_CHAR)
-        strcat(buffer, "char ");
-      else if(flags & FLAGS_FLOATE)
-        strcat(buffer, "floate ");
-      else if(flags & FLAGS_FLOATG)
-        strcat(buffer, "floatg ");
-    }
-    printf("REPORT: %d. %s [%s]\n", i, type, buffer);
-
-  }
-
-
-}
-#endif
-
 /******************************************************************
  *
  * Pass 1:
@@ -537,10 +442,6 @@ static long dprintf_Pass1(const char *format, va_stack_t *vto, char **endpos,
     }
   }
 
-#ifdef DPRINTF_DEBUG2
-  dprintf_Pass1Report(vto, max_param);
-#endif
-
   /* Read the arg list parameters into our data list */
   for(i=0; i<max_param; i++) {
     if((i + 1 < max_param) && (vto[i + 1].type == FORMAT_WIDTH)) {
@@ -919,7 +820,7 @@ static int dprintf_formatf(
     case FORMAT_DOUBLE:
       {
         char formatbuf[32]="%";
-        char *fptr;
+        char *fptr = &formatbuf[1];
         size_t left = sizeof(formatbuf)-strlen(formatbuf);
         int len;
 
@@ -936,15 +837,15 @@ static int dprintf_formatf(
           prec = (long)vto[p->precision].data.num.as_signed;
 
         if(p->flags & FLAGS_LEFT)
-          strcat(formatbuf, "-");
+          *fptr++ = '-';
         if(p->flags & FLAGS_SHOWSIGN)
-          strcat(formatbuf, "+");
+          *fptr++ = '+';
         if(p->flags & FLAGS_SPACE)
-          strcat(formatbuf, " ");
+          *fptr++ = ' ';
         if(p->flags & FLAGS_ALT)
-          strcat(formatbuf, "#");
+          *fptr++ = '#';
 
-        fptr=&formatbuf[strlen(formatbuf)];
+        *fptr = 0;
 
         if(width >= 0) {
           /* RECURSIVE USAGE */
@@ -969,8 +870,8 @@ static int dprintf_formatf(
 
         *fptr = 0; /* and a final zero termination */
 
-        /* NOTE NOTE NOTE!! Not all sprintf() implementations returns number
+        /* NOTE NOTE NOTE!! Not all sprintf implementations return number of
-           of output characters */
+           output characters */
         (sprintf)(work, formatbuf, p->data.dnum);
 
         for(fptr=work; *fptr; fptr++)

src/tool_dirhie.c

@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2013, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -98,12 +98,14 @@ CURLcode create_dir_hierarchy(const char *outfile, FILE *errors)
   char *outdup;
   char *dirbuildup;
   CURLcode result = CURLE_OK;
+  size_t outlen;
 
+  outlen = strlen(outfile);
   outdup = strdup(outfile);
   if(!outdup)
     return CURLE_OUT_OF_MEMORY;
 
-  dirbuildup = malloc(strlen(outfile) + 1);
+  dirbuildup = malloc(outlen + 1);
   if(!dirbuildup) {
     Curl_safefree(outdup);
     return CURLE_OUT_OF_MEMORY;
@@ -119,12 +121,12 @@ CURLcode create_dir_hierarchy(const char *outfile, FILE *errors)
     if(tempdir2 != NULL) {
       size_t dlen = strlen(dirbuildup);
       if(dlen)
-        sprintf(&dirbuildup[dlen], "%s%s", DIR_CHAR, tempdir);
+        snprintf(&dirbuildup[dlen], outlen - dlen, "%s%s", DIR_CHAR, tempdir);
       else {
         if(0 != strncmp(outdup, DIR_CHAR, 1))
           strcpy(dirbuildup, tempdir);
         else
-          sprintf(dirbuildup, "%s%s", DIR_CHAR, tempdir);
+          snprintf(dirbuildup, outlen, "%s%s", DIR_CHAR, tempdir);
       }
       if(access(dirbuildup, F_OK) == -1) {
         if(-1 == mkdir(dirbuildup,(mode_t)0000750)) {

src/tool_operate.c

@@ -805,18 +805,18 @@ int operate(struct Configurable *config, int argc, argv_item_t argv[])
           /*
            * Then append ? followed by the get fields to the url.
            */
-          urlbuffer = malloc(strlen(this_url) + strlen(httpgetfields) + 3);
-          if(!urlbuffer) {
-            res = CURLE_OUT_OF_MEMORY;
-            goto show_error;
-          }
           if(pc)
-            sprintf(urlbuffer, "%s%c%s", this_url, sep, httpgetfields);
+            urlbuffer = aprintf("%s%c%s", this_url, sep, httpgetfields);
           else
             /* Append  / before the ? to create a well-formed url
                if the url contains a hostname only
             */
-            sprintf(urlbuffer, "%s/?%s", this_url, httpgetfields);
+            urlbuffer = aprintf("%s/?%s", this_url, httpgetfields);
+
+          if(!urlbuffer) {
+            res = CURLE_OUT_OF_MEMORY;
+            goto show_error;
+          }
 
           Curl_safefree(this_url); /* free previous URL */
           this_url = urlbuffer; /* use our new URL instead! */

src/tool_operhlp.c

@@ -123,22 +123,20 @@ char *add_file_name_to_url(CURL *curl, char *url, const char *filename)
     /* URL encode the file name */
     encfile = curl_easy_escape(curl, filep, 0 /* use strlen */);
     if(encfile) {
-      char *urlbuffer = malloc(strlen(url) + strlen(encfile) + 3);
+      char *urlbuffer;
-      if(!urlbuffer) {
-        curl_free(encfile);
-        Curl_safefree(url);
-        return NULL;
-      }
       if(ptr)
         /* there is a trailing slash on the URL */
-        sprintf(urlbuffer, "%s%s", url, encfile);
+        urlbuffer = aprintf("%s%s", url, encfile);
       else
         /* there is no trailing slash on the URL */
-        sprintf(urlbuffer, "%s/%s", url, encfile);
+        urlbuffer = aprintf("%s/%s", url, encfile);
 
       curl_free(encfile);
       Curl_safefree(url);
 
+      if(!urlbuffer)
+        return NULL;
+
       url = urlbuffer; /* use our new URL instead! */
     }
   }

src/tool_parsecfg.c

@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2013, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -275,32 +275,33 @@ static char *my_get_line(FILE *fp)
 {
   char buf[4096];
   char *nl = NULL;
-  char *retval = NULL;
+  char *line = NULL;
 
   do {
     if(NULL == fgets(buf, sizeof(buf), fp))
       break;
-    if(!retval) {
+    if(!line) {
-      retval = strdup(buf);
+      line = strdup(buf);
-      if(!retval)
+      if(!line)
         return NULL;
     }
     else {
       char *ptr;
-      ptr = realloc(retval, strlen(retval) + strlen(buf) + 1);
+      size_t linelen = strlen(line);
+      ptr = realloc(line, linelen + strlen(buf) + 1);
       if(!ptr) {
-        Curl_safefree(retval);
+        Curl_safefree(line);
         return NULL;
       }
-      retval = ptr;
+      line = ptr;
-      strcat(retval, buf);
+      strcpy(&line[linelen], buf);
     }
-    nl = strchr(retval, '\n');
+    nl = strchr(line, '\n');
   } while(!nl);
 
   if(nl)
     *nl = '\0';
 
-  return retval;
+  return line;
 }
 

src/tool_setopt.c

@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2013, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -199,7 +199,7 @@ static char *c_escape(const char *str)
       e += 2;
     }
     else if(! isprint(c)) {
-      sprintf(e, "\\%03o", c);
+      snprintf(e, 4, "\\%03o", c);
       e += 4;
     }
     else
@@ -270,7 +270,7 @@ CURLcode tool_setopt_flags(CURL *curl, struct Configurable *config,
         if(!rest)
           break;                /* handled them all */
         /* replace with all spaces for continuation line */
-        sprintf(preamble, "%*s", strlen(preamble), "");
+        snprintf(preamble, sizeof(preamble), "%*s", strlen(preamble), "");
       }
     }
     /* If any bits have no definition, output an explicit value.
@@ -313,7 +313,7 @@ CURLcode tool_setopt_bitmask(CURL *curl, struct Configurable *config,
         if(!rest)
           break;                /* handled them all */
         /* replace with all spaces for continuation line */
-        sprintf(preamble, "%*s", strlen(preamble), "");
+        snprintf(preamble, sizeof(preamble), "%*s", strlen(preamble), "");
       }
     }
     /* If any bits have no definition, output an explicit value.