Going through possible security problems with a fine comb. If you

want to help, check out the current source, and run check_sec.sh.

21 files changed, 51 insertions, 34 deletions

diff --git a/browser.c b/browser.c
index 2c61a873..486b0d36 100644
--- a/browser.c
+++ b/browser.c
@@ -573,7 +573,7 @@ void _mutt_select_file (char *f, size_t flen, int flags, char ***files, int *num
     else
     {
       if (f[0] == '/')
-	strcpy (LastDir, "/");
+	strcpy (LastDir, "/");		/* __STRCPY_CHECKED__ */
       else
 	getcwd (LastDir, sizeof (LastDir));
     }
diff --git a/buffy.c b/buffy.c
index a8352e87..cb5e35fb 100644
--- a/buffy.c
+++ b/buffy.c
@@ -424,7 +424,7 @@ int mutt_buffy_notify (void)
  * given a folder name, this routine gives the next incoming folder with new
  * new mail.
  */
-void mutt_buffy (char *s)
+void mutt_buffy (char *s, size_t slen)
 {
   int count;
   BUFFY *tmp = Incoming;
@@ -447,7 +447,7 @@ void mutt_buffy (char *s)
       mutt_buffy_check (1); /* buffy was wrong - resync things */
       break;
     }
-    strcpy (s, tmp->path);
+    strfcpy (s, tmp->path, slen);
     mutt_pretty_mailbox (s);
     break;
 
@@ -473,7 +473,7 @@ void mutt_buffy (char *s)
       mutt_buffy_check (1); /* buffy was wrong - resync things */
       break;
     }
-    strcpy (s, tmp->path);
+    strfcpy (s, tmp->path, slen);
     mutt_pretty_mailbox (s);
     break;
   }
diff --git a/check_sec.sh b/check_sec.sh
index b1a505a8..d0c1b2f7 100755
--- a/check_sec.sh
+++ b/check_sec.sh
@@ -4,13 +4,24 @@
 # grep for some things which may look like security problems.
 #
 
-TMPFILE="`mktemp fopen.XXXXXX`" || exit 1
-grep -n '\<fopen.*".*w' *.c */*.c | fgrep -v __FOPEN_CHECKED__  > $TMPFILE
-test -s $TMPFILE && {
-	echo "WARNING: UNCHECKED FOPEN CALLS FOUND" ;
-	cat $TMPFILE ;
-	exit 1;
+TMPFILE="`mktemp check_sec.tmp.XXXXXX`" || exit 1
+
+do_check ()
+{
+	egrep -n "$1" *.c */*.c | fgrep -v $2 > $TMPFILE
+	test -s $TMPFILE && {
+		echo "$3" ;
+		cat $TMPFILE;
+		exit 1;
+	}
 }
 
+
+
+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."
+
 rm -f $TMPFILE
 exit 0
diff --git a/curs_main.c b/curs_main.c
index f4972783..2bb165a9 100644
--- a/curs_main.c
+++ b/curs_main.c
@@ -947,7 +947,7 @@ int mutt_index_menu (void)
           cp = _("Open mailbox");
 
 	buf[0] = '\0';
-	mutt_buffy (buf);
+	mutt_buffy (buf, sizeof (buf));
 
 	if (mutt_enter_fname (cp, buf, sizeof (buf), &menu->redraw, 1) == -1)
 	  break;
diff --git a/enter.c b/enter.c
index 50dca39a..13a54c38 100644
--- a/enter.c
+++ b/enter.c
@@ -443,7 +443,7 @@ int _mutt_enter_string (char *buf, size_t buflen, int y, int x,
 	  {
 	    first = 1; /* clear input if user types a real key later */
 	    my_wcstombs (buf, buflen, state->wbuf, state->curpos);
-	    mutt_buffy (buf);
+	    mutt_buffy (buf, buflen);
 	    state->curpos = state->lastchar = my_mbstowcs (&state->wbuf, &state->wbuflen, 0, buf);
 	    break;
 	  }
diff --git a/imap/auth_cram.c b/imap/auth_cram.c
index f41ef4c5..54330c01 100644
--- a/imap/auth_cram.c
+++ b/imap/auth_cram.c
@@ -33,7 +33,7 @@ static void hmac_md5 (const char* password, char* challenge,
 /* imap_auth_cram_md5: AUTH=CRAM-MD5 support. */
 imap_auth_res_t imap_auth_cram_md5 (IMAP_DATA* idata)
 {
-  char ibuf[LONG_STRING], obuf[LONG_STRING];
+  char ibuf[LONG_STRING*4+10], obuf[LONG_STRING];
   unsigned char hmac_response[MD5_DIGEST_LEN];
   int len;
   int rc;
@@ -98,8 +98,12 @@ imap_auth_res_t imap_auth_cram_md5 (IMAP_DATA* idata)
     hmac_response[12], hmac_response[13], hmac_response[14], hmac_response[15]);
   dprint(2, (debugfile, "CRAM response: %s\n", obuf));
 
+  /* XXX - ibuf must be long enough to store the base64 encoding of obuf, 
+   * plus the additional debris
+   */
+  
   mutt_to_base64 ((unsigned char*) ibuf, (unsigned char*) obuf, strlen (obuf));
-  strcpy (ibuf + strlen (ibuf), "\r\n");
+  strcat (ibuf, "\r\n");	/* __STRCAT_CHECKED__ */
   mutt_socket_write (idata->conn, ibuf);
 
   do
diff --git a/imap/auth_gss.c b/imap/auth_gss.c
index b1f03a37..9723cefe 100644
--- a/imap/auth_gss.c
+++ b/imap/auth_gss.c
@@ -136,7 +136,7 @@ imap_auth_res_t imap_auth_gss (IMAP_DATA* idata)
     mutt_to_base64 ((unsigned char*) buf1, send_token.value,
       send_token.length);
     gss_release_buffer (&min_stat, &send_token);
-    strcpy (buf1 + strlen (buf1), "\r\n");
+    strcat (buf1, "\r\n");
     mutt_socket_write (idata->conn, buf1);
 
     if (maj_stat == GSS_S_CONTINUE_NEEDED)
diff --git a/init.c b/init.c
index 6c8fc8bb..12ce342e 100644
--- a/init.c
+++ b/init.c
@@ -262,7 +262,7 @@ int mutt_extract_token (BUFFER *dest, BUFFER *tok, int flags)
 	tok->dsize = expnlen + mutt_strlen (tok->dptr) + 1;
 	ptr = safe_malloc (tok->dsize);
 	memcpy (ptr, expn.data, expnlen);
-	strcpy (ptr + expnlen, tok->dptr);
+	strcpy (ptr + expnlen, tok->dptr);	/* __STRCPY_CHECKED__ */
 	if (tok->destroy)
 	  FREE (&tok->data);
 	tok->data = ptr;
diff --git a/intl/cat-compat.c b/intl/cat-compat.c
index f129f964..7a25fcfd 100644
--- a/intl/cat-compat.c
+++ b/intl/cat-compat.c
@@ -109,14 +109,14 @@ textdomain (domainname)
   if (new_name == NULL)
     return NULL;
 
-  strcpy (new_name, PACKAGE);
+  strcpy (new_name, PACKAGE);	/* __STRCPY_CHECKED__ */
   new_catalog = catopen (new_name, 0);
 
   if (new_catalog == (nl_catd) -1)
     {
       /* NLSPATH search didn't work, try absolute path */
       sprintf (new_name, "%s/%s/LC_MESSAGES/%s.cat", LOCALEDIR, lang,
-	       PACKAGE);
+	       PACKAGE);	/* __SPRINTF_CHECKED__ - sort of... */
       new_catalog = catopen (new_name, 0);
 
       if (new_catalog == (nl_catd) -1)
diff --git a/main.c b/main.c
index babd044c..6507dd79 100644
--- a/main.c
+++ b/main.c
@@ -788,7 +788,7 @@ int main (int argc, char **argv)
 	exit (1);
       }
       folder[0] = 0;
-      mutt_buffy (folder);
+      mutt_buffy (folder, sizeof (folder));
     }
     else if (flags & M_SELECT)
     {
diff --git a/makedoc.c b/makedoc.c
index b5e44446..c5197959 100644
--- a/makedoc.c
+++ b/makedoc.c
@@ -517,10 +517,10 @@ static void char_to_escape (char *dest, unsigned int c)
 {
   switch (c)
   {
-    case '\r': strcpy (dest, "\\r"); break;
-    case '\n': strcpy (dest, "\\n"); break;
-    case '\t': strcpy (dest, "\\t"); break;
-    case '\f': strcpy (dest, "\\f"); break;
+    case '\r': strcpy (dest, "\\r"); break;	/* __STRCPY_CHECKED__ */
+    case '\n': strcpy (dest, "\\n"); break;	/* __STRCPY_CHECKED__ */
+    case '\t': strcpy (dest, "\\t"); break;	/* __STRCPY_CHECKED__ */
+    case '\f': strcpy (dest, "\\f"); break;	/* __STRCPY_CHECKED__ */
     default: sprintf (dest, "\\%03o", c); break;
   }
 }
diff --git a/mkjtags.c b/mkjtags.c
index a8106c0d..68532509 100644
--- a/mkjtags.c
+++ b/mkjtags.c
@@ -77,7 +77,9 @@ void doit (const char *fname, char *prefix, int crlf_pending)
     {
       if ((cp = strrchr (buffer, ',')))
 	*cp = 0;
-      strcpy (tmpf, buffer);
+      strcpy (tmpf, buffer);	/* __STRCPY_CHECKED__ - this program isn't invoked
+				 * with unknown data anyway, so we don't care about
+				 * buffer overflows. */
       
       if ((cp = strrchr (buffer, '/')))
 	*cp = 0;
diff --git a/mutt_sasl.c b/mutt_sasl.c
index 0bd24917..f896c4fa 100644
--- a/mutt_sasl.c
+++ b/mutt_sasl.c
@@ -238,7 +238,7 @@ static int mutt_sasl_cb_pass (sasl_conn_t* conn, void* context, int id,
 
   *psecret = (sasl_secret_t*) malloc (sizeof (sasl_secret_t) + len);
   (*psecret)->len = len;
-  strcpy ((*psecret)->data, account->pass);
+  strcpy ((*psecret)->data, account->pass);	/* __STRCPY_CHECKED__ */
 
   return SASL_OK;
 }
diff --git a/pgp.c b/pgp.c
index 53b8e9fe..d891e71f 100644
--- a/pgp.c
+++ b/pgp.c
@@ -892,7 +892,7 @@ BODY *pgp_decrypt_part (BODY *a, STATE *s, FILE *fpout)
   {
     len = mutt_strlen (buf);
     if (len > 1 && buf[len - 2] == '\r')
-      strcpy (buf + len - 2, "\n");
+      strcpy (buf + len - 2, "\n");	/* __STRCPY_CHECKED__ */
     fputs (buf, fpout);
   }
 
diff --git a/pgpinvoke.c b/pgpinvoke.c
index 30e0b2e7..42a4b6eb 100644
--- a/pgpinvoke.c
+++ b/pgpinvoke.c
@@ -337,7 +337,7 @@ pid_t pgp_invoke_list_keys (FILE **pgpin, FILE **pgpout, FILE **pgperr,
   for (; hints; hints = hints->next)
   {
     snprintf (tmpuids, sizeof (tmpuids), "%s %s", uids, (char *) hints->data);
-    strcpy (uids, tmpuids);
+    strcpy (uids, tmpuids);	/* __STRCPY_CHECKED__ */
   }
 
   return pgp_invoke (pgpin, pgpout, pgperr, pgpinfd, pgpoutfd, pgperrfd,
diff --git a/pgpkey.c b/pgpkey.c
index b197f0b5..6c365a9e 100644
--- a/pgpkey.c
+++ b/pgpkey.c
@@ -603,7 +603,7 @@ pgp_key_t *pgp_ask_for_key (char *tag, char *whatfor,
     for (l = id_defaults; l; l = l->next)
       if (!mutt_strcasecmp (whatfor, l->what))
       {
-	strcpy (resp, NONULL (l->dflt));
+	strfcpy (resp, NONULL (l->dflt), sizeof (resp));
 	break;
       }
   }
diff --git a/pop.c b/pop.c
index 87c2553b..096b3a6d 100644
--- a/pop.c
+++ b/pop.c
@@ -542,10 +542,10 @@ void pop_fetch_mail (void)
   url = p = safe_calloc (strlen (PopHost) + 6, sizeof (char));
   if (url_check_scheme (PopHost) == U_UNKNOWN)
   {
-    strcpy (url, "pop://");
+    strcpy (url, "pop://");	/* __STRCPY_CHECKED__ */
     p = strchr (url, '\0');
   }
-  strcpy (p, PopHost);
+  strcpy (p, PopHost);		/* __STRCPY_CHECKED__ */
 
   if (pop_parse_path (url, &acct))
   {
diff --git a/protos.h b/protos.h
index 832cbf46..f293dc19 100644
--- a/protos.h
+++ b/protos.h
@@ -143,7 +143,7 @@ void mutt_block_signals (void);
 void mutt_block_signals_system (void);
 void mutt_body_handler (BODY *, STATE *);
 void mutt_bounce_message (FILE *fp, HEADER *, ADDRESS *);
-void mutt_buffy (char *);
+void mutt_buffy (char *, size_t);
 void mutt_canonical_charset (char *, size_t, const char *);
 void mutt_check_rescore (CONTEXT *);
 void mutt_clear_error (void);
diff --git a/regex.c b/regex.c
index b7f79577..36bf7b3d 100644
--- a/regex.c
+++ b/regex.c
@@ -5721,7 +5721,7 @@ regerror (errcode, preg, errbuf, errbuf_size)
           errbuf[errbuf_size - 1] = 0;
         }
       else
-        strcpy (errbuf, msg);
+        strcpy (errbuf, msg);	/* __STRCPY_CHECKED__ */
     }
 
   return msg_size;
diff --git a/rfc2231.c b/rfc2231.c
index 8e867b5c..35d39564 100644
--- a/rfc2231.c
+++ b/rfc2231.c
@@ -281,7 +281,7 @@ static void rfc2231_join_continuations (PARAMETER **head,
       vl = strlen (par->value);
       
       safe_realloc ((void **) &value, l + vl + 1);
-      strcpy (value + l, par->value);
+      strcpy (value + l, par->value);	/* __STRCPY_CHECKED__ */
       l += vl;
 
       q = par->next;
diff --git a/sendlib.c b/sendlib.c
index ac0ffb64..0efaf938 100644
--- a/sendlib.c
+++ b/sendlib.c
@@ -2002,7 +2002,7 @@ char *mutt_append_string (char *a, const char *b)
 {
   size_t la = mutt_strlen (a);
   safe_realloc ((void **) &a, la + mutt_strlen (b) + 1);
-  strcpy (a + la, b);
+  strcpy (a + la, b);	/* __STRCPY_CHECKED__ */
   return (a);
 }