Unchecked sprintf/strcat?

To: Mutt Developers <mutt-dev@xxxxxxxx>
Subject: Unchecked sprintf/strcat?
From: TAKAHASHI Tamotsu <ttakah@xxxxxxxxxxxxxxxxx>
Date: Sun, 21 May 2006 18:35:46 +0900
List-unsubscribe: <mailto:mutt-dev-request@mutt.org?Subject="Unsubscribe Mutt Dev"?body=unsubscribe>
Mail-followup-to: Mutt Developers <mutt-dev@xxxxxxxx>
Sender: owner-mutt-dev@xxxxxxxx
User-agent: Mutt/1.5.11

I've found a few unchecked sprintf and strcat.
They don't look very dangerous, but you may want to fix them.

crypt-gpgme.c: sizeof(helpstr) < sizeof(buf)

|   helpstr[0] = 0;
|   mutt_make_help (buf, sizeof (buf), _("Exit  "), menu_to_use, OP_EXIT);
|-  strcat (helpstr, buf);      /* __STRCAT_CHECKED__ */
|+  safe_strcat (helpstr, sizeof (helpstr), buf);

pgpkeys.c: ditto

keymap.c: unchecked sprintf

|   else if (c >= KEY_F0 && c < KEY_F(256)) /* this maximum is just a guess */
|-    sprintf (buf, "<F%d>", c - KEY_F0);
|+    snprintf (buf, sizeof (buf), "<F%d>", c - KEY_F0);


pop_auth.c: ditto

|-    sprintf (hash + 2 * i, "%02x", digest[i]);
|+    snprintf (hash + 2 * i, sizeof (hash) - (2 * i), "%02x", digest[i]);
 
rfc2231.c: ditto

|-      sprintf (t, "%%%02X", (unsigned char)*s);
|+      snprintf (t, elen - (t - e), "%%%02X", (unsigned char)*s);

smime.c: no problem

|       fname = safe_malloc(13); /* Hash + '.' + Suffix + \0 */
|-      sprintf(fname, "%.8x.%i", Table[cur].hash, Table[cur].suffix);
|+      snprintf(fname, 13, "%.8x.%i", Table[cur].hash, Table[cur].suffix);

imap/imap.c:

|-        sprintf (uidbuf, "/%u", HEADER_DATA(h)->uid);
|+        snprintf (uidbuf, sizeof (uidbuf), "/%u", HEADER_DATA(h)->uid);

|-      sprintf (uidbuf, "/%u", HEADER_DATA(h)->uid);
|+      snprintf (uidbuf, sizeof (uidbuf), "/%u", HEADER_DATA(h)->uid);


imap/message.c:

|-        sprintf(uid_buf, "/%u", h.data->uid); /* XXX --tg 21:41 04-07-11 */
|+        snprintf(uid_buf, sizeof (uid_buf), "/%u", h.data->uid); /* XXX --tg 
21:41 04-07-11 */

|-      sprintf(uid_buf, "/%u", h.data->uid);
|+      snprintf(uid_buf, sizeof (uid_buf), "/%u", h.data->uid);
 

-- 
tamo

Attachment: patch-1.5.11cvs.tamo.secwarnings.1.gz
Description: application/gunzip

Index: browser.c
===================================================================
RCS file: /home/roessler/cvs/mutt/browser.c,v
retrieving revision 3.19
diff -p -u -r3.19 browser.c
--- browser.c   6 Oct 2005 06:15:00 -0000       3.19
+++ browser.c   20 May 2006 13:51:42 -0000
@@ -695,7 +695,7 @@ void _mutt_select_file (char *f, size_t 
            if (mutt_strcmp (state.entry[menu->current].name, "..") == 0)
            {
              if (mutt_strcmp ("..", LastDir + mutt_strlen (LastDir) - 2) == 0)
-               strcat (LastDir, "/..");        /* __STRCAT_CHECKED__ */
+               safe_strcat (LastDir, sizeof (LastDir), "/..");
              else
              {
                char *p = strrchr (LastDir + 1, '/');
@@ -707,7 +707,7 @@ void _mutt_select_file (char *f, size_t 
                  if (LastDir[0] == '/')
                    LastDir[1] = 0;
                  else
-                   strcat (LastDir, "/..");    /* __STRCAT_CHECKED__ */
+                   safe_strcat (LastDir, sizeof (LastDir), "/..");
                }
              }
            }
Index: crypt-gpgme.c
===================================================================
RCS file: /home/roessler/cvs/mutt/crypt-gpgme.c,v
retrieving revision 3.10
diff -p -u -r3.10 crypt-gpgme.c
--- crypt-gpgme.c       21 Oct 2005 04:35:37 -0000      3.10
+++ crypt-gpgme.c       20 May 2006 13:51:45 -0000
@@ -3560,15 +3562,15 @@ static crypt_key_t *crypt_select_key (cr
 
   helpstr[0] = 0;
   mutt_make_help (buf, sizeof (buf), _("Exit  "), menu_to_use, OP_EXIT);
-  strcat (helpstr, buf);       /* __STRCAT_CHECKED__ */
+  safe_strcat (helpstr, sizeof (helpstr), buf);
   mutt_make_help (buf, sizeof (buf), _("Select  "), menu_to_use,
                  OP_GENERIC_SELECT_ENTRY);
-  strcat (helpstr, buf);       /* __STRCAT_CHECKED__ */
+  safe_strcat (helpstr, sizeof (helpstr), buf);
   mutt_make_help (buf, sizeof (buf), _("Check key  "),
                   menu_to_use, OP_VERIFY_KEY);
-  strcat (helpstr, buf);       /* __STRCAT_CHECKED__ */
+  safe_strcat (helpstr, sizeof (helpstr), buf);
   mutt_make_help (buf, sizeof (buf), _("Help"), menu_to_use, OP_HELP);
-  strcat (helpstr, buf);       /* __STRCAT_CHECKED__ */
+  safe_strcat (helpstr, sizeof (helpstr), buf);
 
   menu = mutt_new_menu ();
   menu->max = i;
Index: keymap.c
===================================================================
RCS file: /home/roessler/cvs/mutt/keymap.c,v
retrieving revision 3.17
diff -p -u -r3.17 keymap.c
--- keymap.c    17 Sep 2005 20:46:10 -0000      3.17
+++ keymap.c    20 May 2006 13:51:49 -0000
@@ -508,7 +508,7 @@ char *km_keyname (int c)
       snprintf (buf, sizeof (buf), "\\%d%d%d", c >> 6, (c >> 3) & 7, c & 7);
   }
   else if (c >= KEY_F0 && c < KEY_F(256)) /* this maximum is just a guess */
-    sprintf (buf, "<F%d>", c - KEY_F0);
+    snprintf (buf, sizeof (buf), "<F%d>", c - KEY_F0);
   else if (IsPrint (c))
     snprintf (buf, sizeof (buf), "%c", (unsigned char) c);
   else
Index: pgpkey.c
===================================================================
RCS file: /home/roessler/cvs/mutt/pgpkey.c,v
retrieving revision 3.11
diff -p -u -r3.11 pgpkey.c
--- pgpkey.c    17 Sep 2005 20:46:11 -0000      3.11
+++ pgpkey.c    20 May 2006 13:51:52 -0000
@@ -512,14 +512,14 @@ static pgp_key_t pgp_select_key (pgp_key
 
   helpstr[0] = 0;
   mutt_make_help (buf, sizeof (buf), _("Exit  "), MENU_PGP, OP_EXIT);
-  strcat (helpstr, buf);       /* __STRCAT_CHECKED__ */
+  safe_strcat (helpstr, sizeof (helpstr), buf);
   mutt_make_help (buf, sizeof (buf), _("Select  "), MENU_PGP,
                  OP_GENERIC_SELECT_ENTRY);
-  strcat (helpstr, buf);       /* __STRCAT_CHECKED__ */
+  safe_strcat (helpstr, sizeof (helpstr), buf);
   mutt_make_help (buf, sizeof (buf), _("Check key  "), MENU_PGP, 
OP_VERIFY_KEY);
-  strcat (helpstr, buf);       /* __STRCAT_CHECKED__ */
+  safe_strcat (helpstr, sizeof (helpstr), buf);
   mutt_make_help (buf, sizeof (buf), _("Help"), MENU_PGP, OP_HELP);
-  strcat (helpstr, buf);       /* __STRCAT_CHECKED__ */
+  safe_strcat (helpstr, sizeof (helpstr), buf);
 
   menu = mutt_new_menu ();
   menu->max = i;
Index: pop_auth.c
===================================================================
RCS file: /home/roessler/cvs/mutt/pop_auth.c,v
retrieving revision 3.7
diff -p -u -r3.7 pop_auth.c
--- pop_auth.c  17 Sep 2005 20:46:11 -0000      3.7
+++ pop_auth.c  20 May 2006 13:51:52 -0000
@@ -192,7 +192,7 @@ static pop_auth_res_t pop_auth_apop (POP
   MD5Final (digest, &mdContext);
 
   for (i = 0; i < sizeof (digest); i++)
-    sprintf (hash + 2 * i, "%02x", digest[i]);
+    snprintf (hash + 2 * i, sizeof (hash) - (2 * i), "%02x", digest[i]);
 
   /* Send APOP command to server */
   snprintf (buf, sizeof (buf), "APOP %s %s\r\n", pop_data->conn->account.user, 
hash);
Index: rfc2231.c
===================================================================
RCS file: /home/roessler/cvs/mutt/rfc2231.c,v
retrieving revision 3.8
diff -p -u -r3.8 rfc2231.c
--- rfc2231.c   18 May 2006 17:35:30 -0000      3.8
+++ rfc2231.c   20 May 2006 13:51:53 -0000
@@ -348,14 +348,15 @@ int rfc2231_encode_string (char **pd)
 
   if (encode)
   {
-    e = safe_malloc (dlen + 2*ext + strlen (charset) + 3);
-    sprintf (e, "%s''", charset);      /* __SPRINTF_CHECKED__ */
+    size_t elen = dlen + 2*ext + strlen (charset) + 3;
+    e = safe_malloc (elen);
+    snprintf (e, elen, "%s''", charset);
     t = e + strlen (e);
     for (s = d, slen = dlen; slen; s++, slen--)
       if (*s < 0x20 || *s >= 0x7f ||
          strchr (MimeSpecials, *s) || strchr ("*'%", *s))
       {
-       sprintf (t, "%%%02X", (unsigned char)*s);
+       snprintf (t, elen - (t - e), "%%%02X", (unsigned char)*s);
        t += 3;
       }
       else
Index: smime.c
===================================================================
RCS file: /home/roessler/cvs/mutt/smime.c,v
retrieving revision 3.48
diff -p -u -r3.48 smime.c
--- smime.c     16 Dec 2005 18:49:40 -0000      3.48
+++ smime.c     20 May 2006 13:51:57 -0000
@@ -465,7 +465,7 @@ char* smime_ask_for_key (char *prompt, c
     }
     if (hash) {
       fname = safe_malloc(13); /* Hash + '.' + Suffix + \0 */
-      sprintf(fname, "%.8x.%i", Table[cur].hash, Table[cur].suffix);
+      snprintf(fname, 13, "%.8x.%i", Table[cur].hash, Table[cur].suffix);
     }
     else fname = NULL;
   
Index: imap/imap.c
===================================================================
RCS file: /home/roessler/cvs/mutt/imap/imap.c,v
retrieving revision 3.81
diff -p -u -r3.81 imap.c
--- imap/imap.c 18 May 2006 18:35:10 -0000      3.81
+++ imap/imap.c 20 May 2006 13:51:58 -0000
@@ -262,7 +262,7 @@ void imap_expunge_mailbox (IMAP_DATA* id
 #if USE_HCACHE
       if (hc)
       {
-        sprintf (uidbuf, "/%u", HEADER_DATA(h)->uid);
+        snprintf (uidbuf, sizeof (uidbuf), "/%u", HEADER_DATA(h)->uid);
         mutt_hcache_delete (hc, uidbuf, imap_hcache_keylen);
       }
 #endif
@@ -1148,7 +1148,7 @@ int imap_sync_mailbox (CONTEXT* ctx, int
 #if USE_HCACHE
     if (hc && h->deleted)
     {
-      sprintf (uidbuf, "/%u", HEADER_DATA(h)->uid);
+      snprintf (uidbuf, sizeof (uidbuf), "/%u", HEADER_DATA(h)->uid);
       mutt_hcache_delete (hc, uidbuf, imap_hcache_keylen);
     }
 #endif
Index: imap/message.c
===================================================================
RCS file: /home/roessler/cvs/mutt/imap/message.c,v
retrieving revision 3.50
diff -p -u -r3.50 message.c
--- imap/message.c      18 May 2006 18:35:10 -0000      3.50
+++ imap/message.c      20 May 2006 13:51:59 -0000
@@ -158,7 +158,7 @@ int imap_read_headers (IMAP_DATA* idata,
         else if (mfhrc < 0)
           break;
 
-        sprintf(uid_buf, "/%u", h.data->uid); /* XXX --tg 21:41 04-07-11 */
+        snprintf(uid_buf, sizeof (uid_buf), "/%u", h.data->uid); /* XXX --tg 
21:41 04-07-11 */
         uid_validity = (unsigned int*)mutt_hcache_fetch (hc, uid_buf, 
&imap_hcache_keylen);
 
         if (uid_validity != NULL && *uid_validity == idata->uid_validity)
@@ -287,7 +287,7 @@ int imap_read_headers (IMAP_DATA* idata,
       ctx->hdrs[msgno]->content->length = h.content_length;
 
 #if USE_HCACHE
-      sprintf(uid_buf, "/%u", h.data->uid);
+      snprintf(uid_buf, sizeof (uid_buf), "/%u", h.data->uid);
       mutt_hcache_store(hc, uid_buf, ctx->hdrs[msgno], idata->uid_validity, 
&imap_hcache_keylen);
 #endif /* USE_HCACHE */

Follow-Ups:
- Re: Unchecked sprintf/strcat?
  - From: Oswald Buddenhagen

Prev by Date: Re: Patch to CVS
Next by Date: [2006-05-21] CVS repository changes
Previous by thread: Re: dev.mutt.org
Next by thread: Re: Unchecked sprintf/strcat?
Index(es):
- Date
- Thread