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 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. -- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA http://www.8t8.us/configs/gpg-key-transition-statement.txt
# HG changeset patch # User Kevin McCarthy <[email protected]> # Date 1443510999 -28800 # Tue Sep 29 15:16:39 2015 +0800 # Node ID b8b1763789721c97713e63c162ab55dbb24287ed # Parent aec82c4dd826f236e5fd2eeb362bb6a9a44882f6 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 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 --- a/parse.c +++ b/parse.c @@ -38,27 +38,32 @@ * lines. ``line'' must point to a dynamically allocated string; it is * increased if more space is required to fit the whole line. */ 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 { if (fgets (buf, *linelen - offset, f) == NULL || /* end of file or */ (ISSPACE (*line) && !offset)) /* end of headers */ { *line = 0; return (line); } - buf += strlen (buf) - 1; + len = strlen (buf); + if (! len) + return (line); + + buf += len - 1; if (*buf == '\n') { /* we did get a full line. remove trailing space */ while (ISSPACE (*buf)) *buf-- = 0; /* we cannot come beyond line's beginning because * it begins with a non-space */ /* check to see if the next line is a continuation line */ diff --git a/smime.c b/smime.c --- a/smime.c +++ b/smime.c @@ -910,16 +910,17 @@ static int smime_handle_cert_email (char *certificate, char *mailbox, int copy, char ***buffer, int *num) { FILE *fpout = NULL, *fperr = NULL; char tmpfname[_POSIX_PATH_MAX]; 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) { mutt_perror (tmpfname); return 1; } mutt_unlink (tmpfname); @@ -949,17 +950,19 @@ fflush (fpout); rewind (fpout); fflush (fperr); rewind (fperr); 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; ret = ret < 0 ? 0 : ret; count++; } if (ret == -1) @@ -977,17 +980,19 @@ { (*num) = count; *buffer = safe_calloc(sizeof(char*), count); count = 0; 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++; } } else if(copy) ret = 2; safe_fclose (&fpout); diff --git a/smtp.c b/smtp.c --- a/smtp.c +++ b/smtp.c @@ -195,19 +195,18 @@ { safe_fclose (&fp); return r; } 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] == '.') { if (mutt_socket_write_d (conn, ".", -1, M_SOCK_LOG_FULL) == -1) { safe_fclose (&fp); return smtp_err_write; }
signature.asc
Description: PGP signature
