Fix oob reads when fgets returns "\0". (closes #3776)

The ticket reported an out of bounds read in mutt_read_rfc822_line()
when a '\0' was embedded on its own line in the headers.  The function
assumed if fgets() didn't return NULL, then the string would have at
least one character.

I scanned the rest of the code and found three other places making the
same assumption for fgets.

Thanks to hanno for finding this with the "american fuzzy lop" tool.

diff --git a/parse.c b/parse.c
index 58c777474d1fceea02bb708a3a16adcbd9805fa4..3098cae350b6d3fd6ffc0175b5277d8b97ea2e5f 100644 (file)
--- a/parse.c
+++ b/parse.c
@@ -43,6 +43,7 @@ char *mutt_read_rfc822_line (FILE *f, char *line, size_t *linelen)
   char *buf = line;
   int ch;
   size_t offset = 0;
+  size_t len = 0;
 
   FOREVER
   {
@@ -53,7 +54,11 @@ char *mutt_read_rfc822_line (FILE *f, char *line, size_t *linelen)
       return (line);
     }
 
-    buf += strlen (buf) - 1;
+    len = mutt_strlen (buf);
+    if (! len)
+      return (line);
+
+    buf += len - 1;
     if (*buf == '\n')
     {
       /* we did get a full line. remove trailing space */

diff --git a/smime.c b/smime.c
index eebde5629adf327dd7832d94ff7720abe0b638b7..72047bcec40bcdf80cb31894d09d71b643cc6f1e 100644 (file)
--- a/smime.c
+++ b/smime.c
@@ -915,6 +915,7 @@ static int smime_handle_cert_email (char *certificate, char *mailbox,
   char email[STRING];
   int ret = -1, count = 0;
   pid_t thepid;
+  size_t len = 0;
 
   mutt_mktemp (tmpfname, sizeof (tmpfname));
   if ((fperr = safe_fopen (tmpfname, "w+")) == NULL)
@@ -954,7 +955,9 @@ static int smime_handle_cert_email (char *certificate, char *mailbox,
 
   while ((fgets (email, sizeof (email), fpout)))
   {
-    *(email + mutt_strlen (email)-1) = '\0';
+    len = mutt_strlen (email);
+    if (len)
+      *(email + len - 1) = '\0';
     if(mutt_strncasecmp (email, mailbox, mutt_strlen (mailbox)) == 0)
       ret=1;
 
@@ -982,7 +985,9 @@ static int smime_handle_cert_email (char *certificate, char *mailbox,
     rewind (fpout);
     while ((fgets (email, sizeof (email), fpout)))
     {
-      *(email + mutt_strlen (email) - 1) = '\0';
+      len = mutt_strlen (email);
+      if (len)
+        *(email + len - 1) = '\0';
       (*buffer)[count] = safe_calloc(1, mutt_strlen (email) + 1);
       strncpy((*buffer)[count], email, mutt_strlen (email));
       count++;

diff --git a/smtp.c b/smtp.c
index a718006bd82f9fe5a268deb685ea0e6ed81c5397..03054b4bd2415ffdf999eb0d0c100c8f1786ceba 100644 (file)
--- a/smtp.c
+++ b/smtp.c
@@ -200,9 +200,8 @@ smtp_data (CONNECTION * conn, const char *msgfile)
   while (fgets (buf, sizeof (buf) - 1, fp))
   {
     buflen = mutt_strlen (buf);
-    term = buf[buflen-1] == '\n';
-    if (buflen && buf[buflen-1] == '\n'
-       && (buflen == 1 || buf[buflen - 2] != '\r'))
+    term = buflen && buf[buflen-1] == '\n';
+    if (term && (buflen == 1 || buf[buflen - 2] != '\r'))
       snprintf (buf + buflen - 1, sizeof (buf) - buflen + 1, "\r\n");
     if (buf[0] == '.')
     {