]> granicus.if.org Git - neomutt/commitdiff
Handle malformed ms-exchange pgp-encrypted block. (closes #3742)
authorKevin McCarthy <kevin@8t8.us>
Sun, 26 Jul 2015 21:48:53 +0000 (14:48 -0700)
committerKevin McCarthy <kevin@8t8.us>
Sun, 26 Jul 2015 21:48:53 +0000 (14:48 -0700)
In certain circumstances, Exchange corrupts a multipart/encrypted block
into:
  <multipart/mixed>
    <text/plain>
    <application/pgp-encrypted> [BASE64-encoded]
    <application/octet-stream> [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
crypt.c
handler.c
mutt_crypt.h
pgp.c
recvattach.c

index 0062c4c92770c9a07176ee34fdebd5453f42446d..060fc2f2ec5ac80f6570da3d0fc7865f4b560e29 100644 (file)
@@ -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 dc7a669bef97b219729cb1f8b82cae914744a9a0..cec5f88b2ab1d8dbfbe0f5c244c7b0070786ad85 100644 (file)
--- 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:
+ *  <multipart/mixed>
+ *     <text/plain>
+ *     <application/pgp-encrypted> [BASE64-encoded]
+ *     <application/octet-stream> [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;
index 0d71296ca802bae8c6d0e7bd2eb90a00b1a7ff8a..1a062bea88baff6a4fef746d5b74579b0cdee0ee 100644 (file)
--- 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)
   {
index 7a934906ae577855f9c5e4fd89578519abef2eb3..131daccd92c8060dfee1bba7580f96d37b7c703a 100644 (file)
@@ -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 0d5376c047ce979afb058b4c0e3ed74a1defa756..9c03db7cf264caa61bf271e4a0292041c596f8e0 100644 (file)
--- 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)
     {
index 245204974b26f6a7a9d9876d4b04593050ee1605..fddbc2f234377623c73f10ada9b07309bf24be0b 100644 (file)
@@ -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;