From 8c279adb494487f821412d65b4d5fd63cff2a31d Mon Sep 17 00:00:00 2001 From: Kevin McCarthy Date: Sun, 26 Jul 2015 14:48:53 -0700 Subject: [PATCH] Handle malformed ms-exchange pgp-encrypted block. (closes #3742) In certain circumstances, Exchange corrupts a multipart/encrypted block into: [BASE64-encoded] [BASE64-encoded] This patch pulls the full detection of valid/invalid multiparts into mutt_body_handler(). It extracts a run_decode_and_handler() function, which is reused by new intermediate handlers to decode the application/octet-stream part before passing it directly to crypt_pgp_encrypted_handler. These intermediate handlers then check and set any GOODSIG flags back into the parent part. This change may result in less error messages for invalid multipart/encrypted parts. Instead, mutt will default to the multipart_handler if it isn't fully "correct". Viewing attachments uses crypt_pgp_decrypt_mime() which bypasses the handler mechanism. Add decoding to the decrypt_mime() functions for pgp and gpgme. Thanks to Vincent Brillault for his analysis and initial patch. --- crypt-gpgme.c | 91 ++++++++++++++------ crypt.c | 64 +++++++++++++- handler.c | 230 ++++++++++++++++++++++++++++---------------------- mutt_crypt.h | 4 + pgp.c | 98 +++++++++++++-------- recvattach.c | 3 +- 6 files changed, 325 insertions(+), 165 deletions(-) diff --git a/crypt-gpgme.c b/crypt-gpgme.c index 0062c4c92..060fc2f2e 100644 --- a/crypt-gpgme.c +++ b/crypt-gpgme.c @@ -1799,35 +1799,82 @@ int pgp_gpgme_decrypt_mime (FILE *fpin, FILE **fpout, BODY *b, BODY **cur) char tempfile[_POSIX_PATH_MAX]; STATE s; BODY *first_part = b; - int is_signed; + int is_signed = 0; + int need_decode = 0; + int saved_type; + LOFF_T saved_offset; + size_t saved_length; + FILE *decoded_fp = NULL; + int rv = 0; first_part->goodsig = 0; first_part->warnsig = 0; - if(!mutt_is_multipart_encrypted(b)) - return -1; - - if(!b->parts || !b->parts->next) + if (mutt_is_valid_multipart_pgp_encrypted (b)) + b = b->parts->next; + else if (mutt_is_malformed_multipart_pgp_encrypted (b)) + { + b = b->parts->next->next; + need_decode = 1; + } + else return -1; - b = b->parts->next; - memset (&s, 0, sizeof (s)); s.fpin = fpin; + + if (need_decode) + { + saved_type = b->type; + saved_offset = b->offset; + saved_length = b->length; + + mutt_mktemp (tempfile, sizeof (tempfile)); + if ((decoded_fp = safe_fopen (tempfile, "w+")) == NULL) + { + mutt_perror (tempfile); + return (-1); + } + unlink (tempfile); + + fseeko (s.fpin, b->offset, 0); + s.fpout = decoded_fp; + + mutt_decode_attachment (b, &s); + + fflush (decoded_fp); + b->length = ftello (decoded_fp); + b->offset = 0; + rewind (decoded_fp); + s.fpin = decoded_fp; + s.fpout = 0; + } + mutt_mktemp (tempfile, sizeof (tempfile)); if (!(*fpout = safe_fopen (tempfile, "w+"))) { mutt_perror (tempfile); - return -1; + rv = -1; + goto bail; } unlink (tempfile); - *cur = decrypt_part (b, &s, *fpout, 0, &is_signed); + if ((*cur = decrypt_part (b, &s, *fpout, 0, &is_signed)) == NULL) + rv = -1; rewind (*fpout); if (is_signed > 0) first_part->goodsig = 1; - - return *cur? 0:-1; + +bail: + if (need_decode) + { + b->type = saved_type; + b->length = saved_length; + b->offset = saved_offset; + safe_fclose (&decoded_fp); + } + + return rv; } @@ -2519,31 +2566,19 @@ int pgp_gpgme_application_handler (BODY *m, STATE *s) * Implementation of `encrypted_handler'. */ -/* MIME handler for pgp/mime encrypted messages. */ +/* MIME handler for pgp/mime encrypted messages. + * This handler is passed the application/octet-stream directly. + * The caller must propagate a->goodsig to its parent. + */ int pgp_gpgme_encrypted_handler (BODY *a, STATE *s) { char tempfile[_POSIX_PATH_MAX]; FILE *fpout; BODY *tattach; - BODY *orig_body = a; int is_signed; int rc = 0; dprint (2, (debugfile, "Entering pgp_encrypted handler\n")); - a = a->parts; - if (!a || a->type != TYPEAPPLICATION || !a->subtype - || ascii_strcasecmp ("pgp-encrypted", a->subtype) - || !a->next || a->next->type != TYPEAPPLICATION || !a->next->subtype - || ascii_strcasecmp ("octet-stream", a->next->subtype) ) - { - if (s->flags & M_DISPLAY) - state_attach_puts (_("[-- Error: malformed PGP/MIME message! --]\n\n"), - s); - return -1; - } - - /* Move forward to the application/pgp-encrypted body. */ - a = a->next; mutt_mktemp (tempfile, sizeof (tempfile)); if (!(fpout = safe_fopen (tempfile, "w+"))) @@ -2578,7 +2613,7 @@ int pgp_gpgme_encrypted_handler (BODY *a, STATE *s) * status. */ if (mutt_is_multipart_signed (tattach) && !tattach->next) - orig_body->goodsig |= tattach->goodsig; + a->goodsig |= tattach->goodsig; if (s->flags & M_DISPLAY) { diff --git a/crypt.c b/crypt.c index dc7a669be..cec5f88b2 100644 --- a/crypt.c +++ b/crypt.c @@ -314,13 +314,74 @@ int mutt_is_multipart_encrypted (BODY *b) ascii_strcasecmp (p, "application/pgp-encrypted")) return 0; - return PGPENCRYPT; + return PGPENCRYPT; } return 0; } +int mutt_is_valid_multipart_pgp_encrypted (BODY *b) +{ + if (! mutt_is_multipart_encrypted (b)) + return 0; + + b = b->parts; + if (!b || b->type != TYPEAPPLICATION || + !b->subtype || ascii_strcasecmp (b->subtype, "pgp-encrypted")) + return 0; + + b = b->next; + if (!b || b->type != TYPEAPPLICATION || + !b->subtype || ascii_strcasecmp (b->subtype, "octet-stream")) + return 0; + + return PGPENCRYPT; +} + + +/* + * This checks for the malformed layout caused by MS Exchange in + * some cases: + * + * + * [BASE64-encoded] + * [BASE64-encoded] + * See ticket #3742 + */ +int mutt_is_malformed_multipart_pgp_encrypted (BODY *b) +{ + if (!(WithCrypto & APPLICATION_PGP)) + return 0; + + if (!b || b->type != TYPEMULTIPART || + !b->subtype || ascii_strcasecmp (b->subtype, "mixed")) + return 0; + + b = b->parts; + if (!b || b->type != TYPETEXT || + !b->subtype || ascii_strcasecmp (b->subtype, "plain") || + b->length != 0) + return 0; + + b = b->next; + if (!b || b->type != TYPEAPPLICATION || + !b->subtype || ascii_strcasecmp (b->subtype, "pgp-encrypted")) + return 0; + + b = b->next; + if (!b || b->type != TYPEAPPLICATION || + !b->subtype || ascii_strcasecmp (b->subtype, "octet-stream")) + return 0; + + b = b->next; + if (b) + return 0; + + return PGPENCRYPT; +} + + int mutt_is_application_pgp (BODY *m) { int t = 0; @@ -469,6 +530,7 @@ int crypt_query (BODY *m) { t |= mutt_is_multipart_encrypted(m); t |= mutt_is_multipart_signed (m); + t |= mutt_is_malformed_multipart_pgp_encrypted (m); if (t && m->goodsig) t |= GOODSIGN; diff --git a/handler.c b/handler.c index 0d71296ca..1a062bea8 100644 --- a/handler.c +++ b/handler.c @@ -1590,15 +1590,133 @@ static int text_plain_handler (BODY *b, STATE *s) return 0; } -int mutt_body_handler (BODY *b, STATE *s) +static int run_decode_and_handler (BODY *b, STATE *s, handler_t handler, int plaintext) { - int decode = 0; - int plaintext = 0; + int origType; + char *savePrefix = NULL; FILE *fp = NULL; char tempfile[_POSIX_PATH_MAX]; - handler_t handler = NULL; - LOFF_T tmpoffset = 0; size_t tmplength = 0; + LOFF_T tmpoffset = 0; + int decode = 0; + int rc = 0; + + fseeko (s->fpin, b->offset, 0); + + /* see if we need to decode this part before processing it */ + if (b->encoding == ENCBASE64 || b->encoding == ENCQUOTEDPRINTABLE || + b->encoding == ENCUUENCODED || plaintext || + mutt_is_text_part (b)) /* text subtypes may + * require character + * set conversion even + * with 8bit encoding. + */ + { + origType = b->type; + + if (!plaintext) + { + /* decode to a tempfile, saving the original destination */ + fp = s->fpout; + mutt_mktemp (tempfile, sizeof (tempfile)); + if ((s->fpout = safe_fopen (tempfile, "w")) == NULL) + { + mutt_error _("Unable to open temporary file!"); + dprint (1, (debugfile, "Can't open %s.\n", tempfile)); + return -1; + } + /* decoding the attachment changes the size and offset, so save a copy + * of the "real" values now, and restore them after processing + */ + tmplength = b->length; + tmpoffset = b->offset; + + /* if we are decoding binary bodies, we don't want to prefix each + * line with the prefix or else the data will get corrupted. + */ + savePrefix = s->prefix; + s->prefix = NULL; + + decode = 1; + } + else + b->type = TYPETEXT; + + mutt_decode_attachment (b, s); + + if (decode) + { + b->length = ftello (s->fpout); + b->offset = 0; + safe_fclose (&s->fpout); + + /* restore final destination and substitute the tempfile for input */ + s->fpout = fp; + fp = s->fpin; + s->fpin = fopen (tempfile, "r"); + unlink (tempfile); + + /* restore the prefix */ + s->prefix = savePrefix; + } + + b->type = origType; + } + + /* process the (decoded) body part */ + if (handler) + { + rc = handler (b, s); + + if (rc) + { + dprint (1, (debugfile, "Failed on attachment of type %s/%s.\n", TYPE(b), NONULL (b->subtype))); + } + + if (decode) + { + b->length = tmplength; + b->offset = tmpoffset; + + /* restore the original source stream */ + safe_fclose (&s->fpin); + s->fpin = fp; + } + } + s->flags |= M_FIRSTDONE; + + return rc; +} + +static int valid_pgp_encrypted_handler (BODY *b, STATE *s) +{ + int rc; + BODY *octetstream; + + octetstream = b->parts->next; + rc = crypt_pgp_encrypted_handler (octetstream, s); + b->goodsig |= octetstream->goodsig; + + return rc; +} + +static int malformed_pgp_encrypted_handler (BODY *b, STATE *s) +{ + int rc; + BODY *octetstream; + + octetstream = b->parts->next->next; + /* exchange encodes the octet-stream, so re-run it through the decoder */ + rc = run_decode_and_handler (octetstream, s, crypt_pgp_encrypted_handler, 0); + b->goodsig |= octetstream->goodsig; + + return rc; +} + +int mutt_body_handler (BODY *b, STATE *s) +{ + int plaintext = 0; + handler_t handler = NULL; int rc = 0; int oflags = s->flags; @@ -1653,20 +1771,14 @@ int mutt_body_handler (BODY *b, STATE *s) else if (s->flags & M_VERIFY) handler = mutt_signed_handler; } - else if ((WithCrypto & APPLICATION_PGP) - && ascii_strcasecmp ("encrypted", b->subtype) == 0) - { - p = mutt_get_parameter ("protocol", b->parameter); - - if (!p) - mutt_error _("Error: multipart/encrypted has no protocol parameter!"); - else if (ascii_strcasecmp ("application/pgp-encrypted", p) == 0) - handler = crypt_pgp_encrypted_handler; - } + else if (mutt_is_valid_multipart_pgp_encrypted (b)) + handler = valid_pgp_encrypted_handler; + else if (mutt_is_malformed_multipart_pgp_encrypted (b)) + handler = malformed_pgp_encrypted_handler; if (!handler) handler = multipart_handler; - + if (b->encoding != ENC7BIT && b->encoding != ENC8BIT && b->encoding != ENCBINARY) { @@ -1695,90 +1807,7 @@ int mutt_body_handler (BODY *b, STATE *s) option(OPTVIEWATTACH))) && (plaintext || handler)) { - fseeko (s->fpin, b->offset, 0); - - /* see if we need to decode this part before processing it */ - if (b->encoding == ENCBASE64 || b->encoding == ENCQUOTEDPRINTABLE || - b->encoding == ENCUUENCODED || plaintext || - mutt_is_text_part (b)) /* text subtypes may - * require character - * set conversion even - * with 8bit encoding. - */ - { - int origType = b->type; - char *savePrefix = NULL; - - if (!plaintext) - { - /* decode to a tempfile, saving the original destination */ - fp = s->fpout; - mutt_mktemp (tempfile, sizeof (tempfile)); - if ((s->fpout = safe_fopen (tempfile, "w")) == NULL) - { - mutt_error _("Unable to open temporary file!"); - dprint (1, (debugfile, "Can't open %s.\n", tempfile)); - goto bail; - } - /* decoding the attachment changes the size and offset, so save a copy - * of the "real" values now, and restore them after processing - */ - tmplength = b->length; - tmpoffset = b->offset; - - /* if we are decoding binary bodies, we don't want to prefix each - * line with the prefix or else the data will get corrupted. - */ - savePrefix = s->prefix; - s->prefix = NULL; - - decode = 1; - } - else - b->type = TYPETEXT; - - mutt_decode_attachment (b, s); - - if (decode) - { - b->length = ftello (s->fpout); - b->offset = 0; - safe_fclose (&s->fpout); - - /* restore final destination and substitute the tempfile for input */ - s->fpout = fp; - fp = s->fpin; - s->fpin = fopen (tempfile, "r"); - unlink (tempfile); - - /* restore the prefix */ - s->prefix = savePrefix; - } - - b->type = origType; - } - - /* process the (decoded) body part */ - if (handler) - { - rc = handler (b, s); - - if (rc) - { - dprint (1, (debugfile, "Failed on attachment of type %s/%s.\n", TYPE(b), NONULL (b->subtype))); - } - - if (decode) - { - b->length = tmplength; - b->offset = tmpoffset; - - /* restore the original source stream */ - safe_fclose (&s->fpin); - s->fpin = fp; - } - } - s->flags |= M_FIRSTDONE; + rc = run_decode_and_handler (b, s, handler, plaintext); } /* print hint to use attachment menu for disposition == attachment if we're not already being called from there */ @@ -1805,7 +1834,6 @@ int mutt_body_handler (BODY *b, STATE *s) fputs (" --]\n", s->fpout); } - bail: s->flags = oflags | (s->flags & M_FIRSTDONE); if (rc) { diff --git a/mutt_crypt.h b/mutt_crypt.h index 7a934906a..131daccd9 100644 --- a/mutt_crypt.h +++ b/mutt_crypt.h @@ -112,6 +112,10 @@ int mutt_protect (HEADER *, char *); int mutt_is_multipart_encrypted (BODY *); +int mutt_is_valid_multipart_pgp_encrypted (BODY *b); + +int mutt_is_malformed_multipart_pgp_encrypted (BODY *b); + int mutt_is_multipart_signed (BODY *); int mutt_is_application_pgp (BODY *); diff --git a/pgp.c b/pgp.c index 0d5376c04..9c03db7cf 100644 --- a/pgp.c +++ b/pgp.c @@ -954,58 +954,88 @@ int pgp_decrypt_mime (FILE *fpin, FILE **fpout, BODY *b, BODY **cur) char tempfile[_POSIX_PATH_MAX]; STATE s; BODY *p = b; - - if(!mutt_is_multipart_encrypted(b)) - return -1; + int need_decode = 0; + int saved_type; + LOFF_T saved_offset; + size_t saved_length; + FILE *decoded_fp = NULL; + int rv = 0; - if(!b->parts || !b->parts->next) + if (mutt_is_valid_multipart_pgp_encrypted (b)) + b = b->parts->next; + else if (mutt_is_malformed_multipart_pgp_encrypted (b)) + { + b = b->parts->next->next; + need_decode = 1; + } + else return -1; - - b = b->parts->next; - + memset (&s, 0, sizeof (s)); s.fpin = fpin; + + if (need_decode) + { + saved_type = b->type; + saved_offset = b->offset; + saved_length = b->length; + + mutt_mktemp (tempfile, sizeof (tempfile)); + if ((decoded_fp = safe_fopen (tempfile, "w+")) == NULL) + { + mutt_perror (tempfile); + return (-1); + } + unlink (tempfile); + + fseeko (s.fpin, b->offset, 0); + s.fpout = decoded_fp; + + mutt_decode_attachment (b, &s); + + fflush (decoded_fp); + b->length = ftello (decoded_fp); + b->offset = 0; + rewind (decoded_fp); + s.fpin = decoded_fp; + s.fpout = 0; + } + mutt_mktemp (tempfile, sizeof (tempfile)); if ((*fpout = safe_fopen (tempfile, "w+")) == NULL) { mutt_perror (tempfile); - return (-1); + rv = -1; + goto bail; } unlink (tempfile); - *cur = pgp_decrypt_part (b, &s, *fpout, p); - + if ((*cur = pgp_decrypt_part (b, &s, *fpout, p)) == NULL) + rv = -1; rewind (*fpout); - - if (!*cur) - return -1; - - return (0); + +bail: + if (need_decode) + { + b->type = saved_type; + b->length = saved_length; + b->offset = saved_offset; + safe_fclose (&decoded_fp); + } + + return rv; } +/* + * This handler is passed the application/octet-stream directly. + * The caller must propagate a->goodsig to its parent. + */ int pgp_encrypted_handler (BODY *a, STATE *s) { char tempfile[_POSIX_PATH_MAX]; FILE *fpout, *fpin; BODY *tattach; - BODY *p = a; int rc = 0; - - a = a->parts; - if (!a || a->type != TYPEAPPLICATION || !a->subtype || - ascii_strcasecmp ("pgp-encrypted", a->subtype) != 0 || - !a->next || a->next->type != TYPEAPPLICATION || !a->next->subtype || - ascii_strcasecmp ("octet-stream", a->next->subtype) != 0) - { - if (s->flags & M_DISPLAY) - state_attach_puts (_("[-- Error: malformed PGP/MIME message! --]\n\n"), s); - return -1; - } - - /* - * Move forward to the application/pgp-encrypted body. - */ - a = a->next; mutt_mktemp (tempfile, sizeof (tempfile)); if ((fpout = safe_fopen (tempfile, "w+")) == NULL) @@ -1017,7 +1047,7 @@ int pgp_encrypted_handler (BODY *a, STATE *s) if (s->flags & M_DISPLAY) crypt_current_time (s, "PGP"); - if ((tattach = pgp_decrypt_part (a, s, fpout, p)) != NULL) + if ((tattach = pgp_decrypt_part (a, s, fpout, a)) != NULL) { if (s->flags & M_DISPLAY) state_attach_puts (_("[-- The following data is PGP/MIME encrypted --]\n\n"), s); @@ -1035,7 +1065,7 @@ int pgp_encrypted_handler (BODY *a, STATE *s) */ if (mutt_is_multipart_signed (tattach) && !tattach->next) - p->goodsig |= tattach->goodsig; + a->goodsig |= tattach->goodsig; if (s->flags & M_DISPLAY) { diff --git a/recvattach.c b/recvattach.c index 245204974..fddbc2f23 100644 --- a/recvattach.c +++ b/recvattach.c @@ -996,7 +996,8 @@ void mutt_view_attachments (HEADER *hdr) } if ((WithCrypto & APPLICATION_PGP) && (hdr->security & APPLICATION_PGP)) { - if (mutt_is_multipart_encrypted(hdr->content)) + if (mutt_is_multipart_encrypted(hdr->content) || + mutt_is_malformed_multipart_pgp_encrypted(hdr->content)) secured = !crypt_pgp_decrypt_mime (msg->fp, &fp, hdr->content, &cur); else need_secured = 0; -- 2.40.0