safe_strcat, safe_strncat. Thanks to Ulf H. for noting the wrong

use of strncat in part of the mutt code base.

diff --git a/browser.c b/browser.c
index 4dc4cc716928b24ed69cd08b807037a62f5449fc..7b64d85ab25840dcf9a2b9a89875f3d0a9ad6ef2 100644 (file)
--- a/browser.c
+++ b/browser.c
@@ -574,8 +574,8 @@ void _mutt_select_file (char *f, size_t flen, int flags, char ***files, int *num
       else
       {
        getcwd (LastDir, sizeof (LastDir));
-       strcat (LastDir, "/");  /* __STRCAT_CHECKED__ */
-       strncat (LastDir, f, i);
+       safe_strcat (LastDir, sizeof (LastDir), "/");
+       safe_strncat (LastDir, sizeof (LastDir), f, i);
       }
     }
     else

diff --git a/buffy.c b/buffy.c
index 7f24b7c2bc3f34c5028863fcf027c8c95e257389..ac251cf6f2000d26c7d18d7a15914e6fb0c99d5f 100644 (file)
--- a/buffy.c
+++ b/buffy.c
@@ -439,7 +439,7 @@ int mutt_buffy_list (void)
   pos = 0;
   first = 1;
   buffylist[0] = 0;
-  pos += strlen (strncat (buffylist, _("New mail in "), sizeof (buffylist) - 1 - pos));
+  pos += strlen (strncat (buffylist, _("New mail in "), sizeof (buffylist) - 1 - pos)); /* __STRNCAT_CHECKED__ */
   for (tmp = Incoming; tmp; tmp = tmp->next)
   {
     /* Is there new mail in this mailbox? */
@@ -453,21 +453,21 @@ int mutt_buffy_list (void)
       break;
     
     if (!first)
-      pos += strlen (strncat(buffylist + pos, ", ", sizeof(buffylist)-1-pos));
+      pos += strlen (strncat(buffylist + pos, ", ", sizeof(buffylist)-1-pos)); /* __STRNCAT_CHECKED__ */
 
     /* Prepend an asterisk to mailboxes not already notified */
     if (!tmp->notified)
     {
-      /* pos += strlen (strncat(buffylist + pos, "*", sizeof(buffylist)-1-pos)); */
+      /* pos += strlen (strncat(buffylist + pos, "*", sizeof(buffylist)-1-pos));  __STRNCAT_CHECKED__ */
       tmp->notified = 1;
       BuffyNotify--;
     }
-    pos += strlen (strncat(buffylist + pos, path, sizeof(buffylist)-1-pos));
+    pos += strlen (strncat(buffylist + pos, path, sizeof(buffylist)-1-pos)); /* __STRNCAT_CHECKED__ */
     first = 0;
   }
   if (!first && tmp)
   {
-    strncat (buffylist + pos, ", ...", sizeof (buffylist) - 1 - pos);
+    strncat (buffylist + pos, ", ...", sizeof (buffylist) - 1 - pos); /* __STRNCAT_CHECKED__ */
   }
   if (!first)
   {

diff --git a/check_sec.sh b/check_sec.sh
index 5ae2019fe2ceffcdbbb1d0fe5a0fcc08a2292895..4c5047a579ef76792a0b2fdb311c405bbfb17dc1 100755 (executable)
--- a/check_sec.sh
+++ b/check_sec.sh
@@ -34,6 +34,7 @@ do_check '\<fopen.*'\"'.*w' __FOPEN_CHECKED__ "Alert: Unchecked fopen calls."
 do_check '\<(mutt_)?strcpy' __STRCPY_CHECKED__ "Alert: Unchecked strcpy calls."
 do_check '\<strcat' __STRCAT_CHECKED__ "Alert: Unchecked strcat calls."
 do_check '\<sprintf.*%s' __SPRINTF_CHECKED__ "Alert: Unchecked sprintf calls."
+do_check '\<strncat' __STRNCAT_CHECKED__ "You probably meant safe_strcat here."
 
 # don't do this check on others' code.
 do_check_files '\<(malloc|realloc|free|strdup)[        ]*\(' __MEM_CHECKED__ "Alert: Use of traditional memory management calls." \

diff --git a/commands.c b/commands.c
index 95a5334f043eedcc9413ef66de1ade83861203b1..68b7b1819f4ca659159aa17b570597a47711dd64 100644 (file)
--- a/commands.c
+++ b/commands.c
@@ -282,10 +282,10 @@ void ci_bounce_message (HEADER *h, int *redraw)
     mutt_format_string (prompt, sizeof (prompt),
                        0, COLS-extra_space, 0, 0,
                        prompt, sizeof (prompt), 0);
-    strncat (prompt, "...?", sizeof (prompt));
+    safe_strcat (prompt, sizeof (prompt), "...?");
   }
   else
-    strncat (prompt, "?", sizeof (prompt));
+    safe_strcat (prompt, sizeof (prompt), "?");
 
   if (query_quadoption (OPT_BOUNCE, prompt) != M_YES)
   {

diff --git a/edit.c b/edit.c
index e29cb6691efaa61ef4e6bc1b5c25900361bcc03a..c6b9ba76e8bf379953c35f8aa0b64c461fa82e9b 100644 (file)
--- a/edit.c
+++ b/edit.c
@@ -463,7 +463,7 @@ int mutt_builtin_editor (const char *path, HEADER *msg, HEADER *cur)
       done = 1;
     else
     {
-      strncat (tmp, "\n", sizeof(tmp)); tmp[sizeof(tmp) - 1] = '\0';
+      safe_strcat (tmp, sizeof (tmp), "\n");
       if (buflen == bufmax)
        safe_realloc (&buf, sizeof (char *) * (bufmax += 25));
       buf[buflen++] = safe_strdup (tmp[1] == '~' ? tmp + 1 : tmp);

diff --git a/imap/auth_cram.c b/imap/auth_cram.c
index 3003f7aa147cb5b0b976bf1dc02921b341c145cd..3baa3c8c439a535de8d12a577164d0495a4ca136 100644 (file)
--- a/imap/auth_cram.c
+++ b/imap/auth_cram.c
@@ -104,7 +104,7 @@ imap_auth_res_t imap_auth_cram_md5 (IMAP_DATA* idata, const char* method)
   
   mutt_to_base64 ((unsigned char*) ibuf, (unsigned char*) obuf, strlen (obuf),
                  sizeof (ibuf) - 2);
-  strncat (ibuf, "\r\n", sizeof (ibuf));
+  safe_strcat (ibuf, sizeof (ibuf), "\r\n");
   mutt_socket_write (idata->conn, ibuf);
 
   do

diff --git a/imap/auth_gss.c b/imap/auth_gss.c
index a6a290862fbf889032f49704683b287344d596bb..03f48d3ac395143c74692d00a6fa11ad8bf41782 100644 (file)
--- a/imap/auth_gss.c
+++ b/imap/auth_gss.c
@@ -122,7 +122,7 @@ imap_auth_res_t imap_auth_gss (IMAP_DATA* idata, const char* method)
   mutt_to_base64 ((unsigned char*) buf1, send_token.value, send_token.length,
     sizeof (buf1) - 2);
   gss_release_buffer (&min_stat, &send_token);
-  strncat (buf1, "\r\n", sizeof (buf1));
+  safe_strcat (buf1, sizeof (buf1), "\r\n");
   mutt_socket_write (idata->conn, buf1);
 
   while (maj_stat == GSS_S_CONTINUE_NEEDED)
@@ -158,7 +158,7 @@ imap_auth_res_t imap_auth_gss (IMAP_DATA* idata, const char* method)
     mutt_to_base64 ((unsigned char*) buf1, send_token.value,
       send_token.length, sizeof (buf1) - 2);
     gss_release_buffer (&min_stat, &send_token);
-    strncat (buf1, "\r\n", sizeof (buf1));
+    safe_strcat (buf1, sizeof (buf1), "\r\n");
     mutt_socket_write (idata->conn, buf1);
   }
 
@@ -226,7 +226,7 @@ imap_auth_res_t imap_auth_gss (IMAP_DATA* idata, const char* method)
                  sizeof (buf1) - 2);
   dprint (2, (debugfile, "Requesting authorisation as %s\n",
     idata->conn->account.user));
-  strncat (buf1, "\r\n", sizeof (buf1));
+  safe_strcat (buf1, sizeof (buf1), "\r\n");
   mutt_socket_write (idata->conn, buf1);
 
   /* Joy of victory or agony of defeat? */

diff --git a/imap/imap.c b/imap/imap.c
index 2b5d754b0624ec8da3a9aeb4ea62cb71642c8187..ed70258a6a77ca7cb9a1b203e679ed0d11b758db 100644 (file)
--- a/imap/imap.c
+++ b/imap/imap.c
@@ -791,7 +791,7 @@ static void imap_set_flag (IMAP_DATA* idata, int aclbit, int flag,
 {
   if (mutt_bit_isset (idata->rights, aclbit))
     if (flag)
-      strncat (flags, str, flsize);
+      safe_strcat (flags, flsize, str);
 }
 
 /* imap_make_msg_set: make an IMAP4rev1 UID message set out of a set of

diff --git a/imap/message.c b/imap/message.c
index 577d5420149b5b810535490c0df4f408d2def958..4f3abb72de7559a006ebfa92049356fed5a071d3 100644 (file)
--- a/imap/message.c
+++ b/imap/message.c
@@ -663,8 +663,8 @@ void imap_add_keywords (char* s, HEADER* h, LIST* mailbox_flags, size_t slen)
   {
     if (msg_has_flag (mailbox_flags, keywords->data))
     {
-      strncat (s, keywords->data, slen);
-      strncat (s, " ", slen);
+      safe_strcat (s, slen, keywords->data);
+      safe_strcat (s, slen, " ");
     }
     keywords = keywords->next;
   }

diff --git a/lib.c b/lib.c
index f3cb073b9e8d93e3b0417ee8b3fad6d663e8f104..f6be051069a5fed4262ee8e1f017ce4f507520dd 100644 (file)
--- a/lib.c
+++ b/lib.c
@@ -153,6 +153,45 @@ char *safe_strdup (const char *s)
   return (p);
 }
 
+char *safe_strcat (char *d, size_t l, const char *s)
+{
+  char *p = d;
+
+  if (!l) 
+    return d;
+
+  l--; /* Space for the trailing '\0'. */
+  
+  for (; *d && l; l--)
+    d++;
+  for (; *s && l; l--)
+    *d++ = *s++;
+
+  *d = '\0';
+  
+  return p;
+}
+
+char *safe_strncat (char *d, size_t l, const char *s, size_t sl)
+{
+  char *p = d;
+
+  if (!l)
+    return d;
+  
+  l--; /* Space for the trailing '\0'. */
+  
+  for (; *d && l; l--)
+    d++;
+  for (; *s && l && sl; l--, sl--)
+    *d++ = *s++;
+
+  *d = '\0';
+  
+  return p;
+}
+
+
 void mutt_str_replace (char **p, const char *s)
 {
   FREE (p);

diff --git a/lib.h b/lib.h
index 2bda25d2fa06e70b03b3b30457d60af757b8d059..358fa7239c206a55ce08c4e2a463c545639e6f5b 100644 (file)
--- a/lib.h
+++ b/lib.h
@@ -115,6 +115,8 @@ char *mutt_skip_whitespace (char *);
 char *mutt_strlower (char *);
 char *mutt_substrcpy (char *, const char *, const char *, size_t);
 char *mutt_substrdup (const char *, const char *);
+char *safe_strcat (char *, size_t, const char *);
+char *safe_strncat (char *, size_t, const char *, size_t);
 char *safe_strdup (const char *);
 
 const char *mutt_stristr (const char *, const char *);

diff --git a/mutt_ssl.c b/mutt_ssl.c
index 171b7e6734fab2bbeef30c3544ee980b65db748e..2c09e650d99d62c0f15d25f5f3af7085fe4d08c8 100644 (file)
--- a/mutt_ssl.c
+++ b/mutt_ssl.c
@@ -417,7 +417,7 @@ static void x509_fingerprint (char *s, int l, X509 * cert)
     {
       char ch[8];
       snprintf (ch, 8, "%02X%s", md[j], (j % 2 ? " " : ""));
-      strncat (s, ch, l);
+      safe_strcat (s, l, ch);
     }
   }
 }
@@ -629,9 +629,9 @@ static int ssl_check_certificate (sslsockdata * data)
   
   helpstr[0] = '\0';
   mutt_make_help (buf, sizeof (buf), _("Exit  "), MENU_GENERIC, OP_EXIT);
-  strncat (helpstr, buf, sizeof (helpstr));
+  safe_strcat (helpstr, sizeof (helpstr), buf);
   mutt_make_help (buf, sizeof (buf), _("Help"), MENU_GENERIC, OP_HELP);
-  strncat (helpstr, buf, sizeof (helpstr));
+  safe_strcat (helpstr, sizeof (helpstr), buf);
   menu->help = helpstr;
 
   done = 0;

diff --git a/muttlib.c b/muttlib.c
index 395bde1fcb7d79caec96773562f0d46156a3ccff..a0bb414e85b5e53e44f2a0c389126fcc95db352a 100644 (file)
--- a/muttlib.c
+++ b/muttlib.c
@@ -815,8 +815,8 @@ void mutt_expand_fmt (char *dest, size_t destlen, const char *fmt, const char *s
   
   if (!found && destlen > 0)
   {
-    strncat (dest, " ", destlen);
-    strncat (dest, src, destlen-1);
+    safe_strcat (dest, destlen, " ");
+    safe_strcat (dest, destlen, src);
   }
   
 }
@@ -1332,8 +1332,6 @@ BUFFER * mutt_buffer_init(BUFFER *b)
  */
 BUFFER * mutt_buffer_from(BUFFER *b, char *seed)
 {
-  int n;
-
   if (!seed)
     return NULL;
 
@@ -1497,7 +1495,7 @@ int mutt_match_spam_list (const char *s, SPAM_LIST *l, char *text, int x)
     /* If this pattern needs more matches, expand pmatch. */
     if (l->nmatch > nmatch)
     {
-      safe_realloc ((void**) &pmatch, l->nmatch * sizeof(regmatch_t));
+      safe_realloc (&pmatch, l->nmatch * sizeof(regmatch_t));
       nmatch = l->nmatch;
     }
 

diff --git a/recvcmd.c b/recvcmd.c
index 372c3a1679e656e64f14f0461ead6658d0b064b1..ccbf656e07b9550a4e85059ac15f98be29358a3b 100644 (file)
--- a/recvcmd.c
+++ b/recvcmd.c
@@ -180,10 +180,10 @@ void mutt_attach_bounce (FILE * fp, HEADER * hdr,
     mutt_format_string (prompt, sizeof (prompt) - 4,
                        0, COLS-extra_space, 0, 0,
                        prompt, sizeof (prompt), 0);
-    strncat (prompt, "...?", sizeof (prompt));
+    safe_strcat (prompt, sizeof (prompt), "...?");
   }
   else
-    strncat (prompt, "?", sizeof (prompt));
+    safe_strcat (prompt, sizeof (prompt), "?");
 
   if (query_quadoption (OPT_BOUNCE, prompt) != M_YES)
   {

diff --git a/url.c b/url.c
index 5be09dbe8985014e33106a24f8348d4f6ee5d92b..a973bbf8940415ae56eea6f81a87592826b4ff5b 100644 (file)
--- a/url.c
+++ b/url.c
@@ -166,8 +166,11 @@ int url_parse_ciss (ciss_url_t *ciss, char *src)
 }
 
 /* url_ciss_tostring: output the URL string for a given CISS object. */
+
 int url_ciss_tostring (ciss_url_t* ciss, char* dest, size_t len, int flags)
 {
+  long l;
+
   if (ciss->scheme == U_UNKNOWN)
     return -1;
 
@@ -175,25 +178,26 @@ int url_ciss_tostring (ciss_url_t* ciss, char* dest, size_t len, int flags)
 
   if (ciss->host)
   {
-    strncat (dest, "//", len - strlen (dest));
+    safe_strcat (dest, len, "//");
+    len -= (l = strlen (dest)); dest += l;
+    
     if (ciss->user) {
       if (flags & U_DECODE_PASSWD && ciss->pass)
-       snprintf (dest + strlen (dest), len - strlen (dest), "%s:%s@",
-                 ciss->user, ciss->pass);
+       snprintf (dest, len, "%s:%s@", ciss->user, ciss->pass);
       else
-       snprintf (dest + strlen (dest), len - strlen (dest), "%s@",
-                 ciss->user);
+       snprintf (dest, len, "%s@", ciss->user);
+
+      len -= (l = strlen (dest)); dest += l;
     }
 
     if (ciss->port)
-      snprintf (dest + strlen (dest), len - strlen (dest), "%s:%hu/",
-               ciss->host, ciss->port);
+      snprintf (dest, len, "%s:%hu/", ciss->host, ciss->port);
     else
-      snprintf (dest + strlen (dest), len - strlen (dest), "%s/", ciss->host);
+      snprintf (dest, len, "%s/", ciss->host);
   }
 
   if (ciss->path)
-    strncat (dest, ciss->path, len - strlen (dest));
+    safe_strcat (dest, len, ciss->path);
 
   return 0;
 }