From: Thomas Roessler <roessler@does-not-exist.org>
Date: Sat, 30 Oct 2004 21:59:00 +0000 (+0000)
Subject: safe_strcat, safe_strncat.  Thanks to Ulf H. for noting the wrong
X-Git-Tag: mutt-1-5-15-rel~63
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d2a419177d377282a2ad09aff229574a02be223e;p=mutt

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 4dc4cc71..7b64d85a 100644
--- 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 7f24b7c2..ac251cf6 100644
--- 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 5ae2019f..4c5047a5 100755
--- 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 95a5334f..68b7b181 100644
--- 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 e29cb669..c6b9ba76 100644
--- 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 3003f7aa..3baa3c8c 100644
--- 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 a6a29086..03f48d3a 100644
--- 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 2b5d754b..ed70258a 100644
--- 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 577d5420..4f3abb72 100644
--- 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 f3cb073b..f6be0510 100644
--- 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 2bda25d2..358fa723 100644
--- 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 171b7e67..2c09e650 100644
--- 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 395bde1f..a0bb414e 100644
--- 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 372c3a16..ccbf656e 100644
--- 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 5be09dbe..a973bbf8 100644
--- 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;
 }