bad boundary choice in forwarded multipart/signed multipart/mixed

bad boundary choice in forwarded multipart/signed multipart/mixed

[Greg Troxel]

[-- Attachment #2: Type: text/plain, Size: 1074 bytes --]


Using gnus from CVS about a month ago I forwarded one of my own signed
messages that itself contained a forwarded message, and a colleague
pointed out that the MIME boundary string had been reused.  That
particular example can't be sent to this list, but I managed to
reproduce the problem with test messages.

The inner test message had boundary =-=-= originally.  That got remapped
whent he message was forwarded, which I suppose is ok.  But it got
remaped to ===-=-= and that's the same value that was chosen for the
outer message's multipart/signed.

The below was generated with current CVS head epg and gnus git as of:

  commit 3aa91c009017b508c0f6298f7e8ee3a622bdccc2
  Author: Katsumi Yamaoka <yamaoka@jpl.org>
  Date:   Tue Apr 27 06:38:57 2010 +0000

      Don't load tm and apel XEmacs packages when compiling.


It looks like the key code is in mml.el in gnus, but I haven't
understood it yet.

Is perhaps the issue that the multipart/signed code is called without
the forwarded heder visible, and then the forwarded header is also
changed to the same value.


[-- Attachment #3: Type: message/rfc822, Size: 1745 bytes --]

[-- Attachment #3.1.1.1: Type: text/plain, Size: 49 bytes --]


forward of multipart/mixed as multipart/signed


[-- Attachment #3.1.1.2: Type: message/rfc822, Size: 88 bytes --]

[-- Attachment #3.1.2: Type: text/plain, Size: 31 bytes --]

Test multipart/mixed message.


[-- Attachment #3.1.3: foo.txt --]
[-- Type: text/plain, Size: 40 bytes --]

test attachment

--===-=-=--

--=-=-=--

[-- Attachment #3.1.4: Type: application/pgp-signature, Size: 194 bytes --]

Re: bad boundary choice in forwarded multipart/signed multipart/mixed

[Andreas Seltenreich]

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

Greg Troxel writes:

> The inner test message had boundary =-=-= originally.  That got remapped
> whent he message was forwarded, which I suppose is ok.

Yeah, Gnus is a bit eager with regenerating MIME (you can tame it using
message-forward-show-mml).

> But it got remaped to ===-=-= and that's the same value that was
> chosen for the outer message's multipart/signed.
[...]
> It looks like the key code is in mml.el in gnus, but I haven't
> understood it yet.

My analysis so far:

(a) mml-multipart-number is let-bound on recursive descend of
    mml-generate-mime.  So mml-compute-boundary potentially generates
    duplicate delimiters unless the collision test prevents it.

(b) The collision test code doesn't look at the content of
    message/rfc822 [#mml] handles.

(c) If it would look at them, it wouldn't note a collision generated by
    (a) since the handle still contains the untranslated
    [#multipart]-tags instead of MIME.

I think (b) needs to be fixed anyway because a collision could occur
independently of (a).  Now either fixing (a) or (c) should prevent the
generation of broken MIME.  I guess squishing the bug the (b) + (c) way
is more adequate since it's less invasive.  Will push in a jiffy.

Thanks for your elaborate report,
andreas


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 933 bytes --]

diff --git a/lisp/mml.el b/lisp/mml.el
index f262dc6..9dc0966 100644
--- a/lisp/mml.el
+++ b/lisp/mml.el
@@ -520,7 +520,10 @@ (defun mml-generate-mime-1 (cont)
 			  ;; `m-g-d-t' will be bound to "message/rfc822"
 			  ;; when encoding an article to be forwarded.
 			  (mml-generate-default-type "text/plain"))
-		      (mml-to-mime))
+		      (mml-to-mime)
+		      ;; Update handle so mml-compute-boundary can
+		      ;; detect collisions with the nested parts.
+		      (setcdr (assoc 'contents cont) (buffer-string)))
 		    (let ((mm-7bit-chars (concat mm-7bit-chars "\x1b")))
 		      ;; ignore 0x1b, it is part of iso-2022-jp
 		      (setq encoding (mm-body-7-or-8))))
@@ -699,7 +702,7 @@ (defun mml-compute-boundary (cont)
 (defun mml-compute-boundary-1 (cont)
   (let (filename)
     (cond
-     ((eq (car cont) 'part)
+     ((member (car cont) '(part mml))
       (with-temp-buffer
 	(cond
 	 ((cdr (assq 'buffer cont))