[PATCH] emacs: Quote MML tags in replies

[Aaron Ecay]

Emacs message-mode uses certain text strings to indicate how to attach
files to outgoing mail.  If these are present in the text of an email,
and a user is tricked into replying to the message, the user’s files
could be exposed.
---

To demonstrate this, open a reply to this message then remove the
exclamation marks after the hash marks below.  Create a file in your
home directory called passwd.  Then press C-u M-x mml-preview.  A
(possibly base64-encoded) version of your ~/passwd file will replace
the following lines:

<#!part type="application/octet-stream" filename="~/passwd"
disposition=attachment description=foo>
<#!/part>

It works equally well (and more dangerously) with /etc/passwd, but I
didn't use that filename here to avoid the danger of someone
accidentally attaching their /etc/passwd to a reply in this thread!

 emacs/notmuch-mua.el |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index d8ab822..c25c6b9 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -115,7 +115,8 @@ list."
     (push-mark))
   (set-buffer-modified-p nil)
 
-  (message-goto-body))
+  (message-goto-body)
+  (mml-quote-region (point) (mark)))
 
 (defun notmuch-mua-forward-message ()
   (message-forward)
--
1.7.8.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Pieter Praet]

On Thu, 19 Jan 2012 13:43:09 -0500, Aaron Ecay <[hidden email]> wrote:

> Emacs message-mode uses certain text strings to indicate how to attach
> files to outgoing mail.  If these are present in the text of an email,
> and a user is tricked into replying to the message, the user’s files
> could be exposed.
> ---
>
> To demonstrate this, open a reply to this message then remove the
> exclamation marks after the hash marks below.  Create a file in your
> home directory called passwd.  Then press C-u M-x mml-preview.  A
> (possibly base64-encoded) version of your ~/passwd file will replace
> the following lines:
>
> <#!part type="application/octet-stream" filename="~/passwd"
> disposition=attachment description=foo>
> <#!/part>
>
> It works equally well (and more dangerously) with /etc/passwd, but I
> didn't use that filename here to avoid the danger of someone
> accidentally attaching their /etc/passwd to a reply in this thread!
>
>  emacs/notmuch-mua.el |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index d8ab822..c25c6b9 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -115,7 +115,8 @@ list."
>      (push-mark))
>    (set-buffer-modified-p nil)
>  
> -  (message-goto-body))
> +  (message-goto-body)
> +  (mml-quote-region (point) (mark)))
>  
>  (defun notmuch-mua-forward-message ()
>    (message-forward)
> --
> 1.7.8.3
>
> _______________________________________________
> notmuch mailing list
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch

Wow, nice catch!  You've just earned yourself a raise!

An urgent +1 !


### OT:
For some reason, `mml-quote-region' explicitly re-quotes
already quoted MML tags:

  "<#!*/?\\(multipart\\|part\\|external\\|mml\\)"

Why is that ?


Peace

--
Pieter
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Austin Clements]

Quoth Pieter Praet on Jan 19 at 11:23 pm:

> On Thu, 19 Jan 2012 13:43:09 -0500, Aaron Ecay <[hidden email]> wrote:
> > Emacs message-mode uses certain text strings to indicate how to attach
> > files to outgoing mail.  If these are present in the text of an email,
> > and a user is tricked into replying to the message, the user’s files
> > could be exposed.
> > ---
> >
> > To demonstrate this, open a reply to this message then remove the
> > exclamation marks after the hash marks below.  Create a file in your
> > home directory called passwd.  Then press C-u M-x mml-preview.  A
> > (possibly base64-encoded) version of your ~/passwd file will replace
> > the following lines:
> >
> > <#!part type="application/octet-stream" filename="~/passwd"
> > disposition=attachment description=foo>
> > <#!/part>
> >
> > It works equally well (and more dangerously) with /etc/passwd, but I
> > didn't use that filename here to avoid the danger of someone
> > accidentally attaching their /etc/passwd to a reply in this thread!
> >
> >  emacs/notmuch-mua.el |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> > index d8ab822..c25c6b9 100644
> > --- a/emacs/notmuch-mua.el
> > +++ b/emacs/notmuch-mua.el
> > @@ -115,7 +115,8 @@ list."
> >      (push-mark))
> >    (set-buffer-modified-p nil)
> >  
> > -  (message-goto-body))
> > +  (message-goto-body)
> > +  (mml-quote-region (point) (mark)))
> >  
> >  (defun notmuch-mua-forward-message ()
> >    (message-forward)
>
> Wow, nice catch!  You've just earned yourself a raise!

Indeed.

> An urgent +1 !
>
>
> ### OT:
> For some reason, `mml-quote-region' explicitly re-quotes
> already quoted MML tags:
>
>   "<#!*/?\\(multipart\\|part\\|external\\|mml\\)"
>
> Why is that ?

Probably so the transformation is invertible, though as far as I can
tell there's no mml-unquote-region.
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Austin Clements]

LGTM and I think it could go in despite my two comments below.

Quoth Aaron Ecay on Jan 19 at  1:43 pm:

> Emacs message-mode uses certain text strings to indicate how to attach
> files to outgoing mail.  If these are present in the text of an email,
> and a user is tricked into replying to the message, the user’s files
> could be exposed.
> ---
>
> To demonstrate this, open a reply to this message then remove the
> exclamation marks after the hash marks below.  Create a file in your
> home directory called passwd.  Then press C-u M-x mml-preview.  A
> (possibly base64-encoded) version of your ~/passwd file will replace
> the following lines:
>
> <#!part type="application/octet-stream" filename="~/passwd"
> disposition=attachment description=foo>
> <#!/part>
>
> It works equally well (and more dangerously) with /etc/passwd, but I
> didn't use that filename here to avoid the danger of someone
> accidentally attaching their /etc/passwd to a reply in this thread!
>
>  emacs/notmuch-mua.el |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index d8ab822..c25c6b9 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -115,7 +115,8 @@ list."
>      (push-mark))
>    (set-buffer-modified-p nil)
>  
> -  (message-goto-body))
> +  (message-goto-body)
> +  (mml-quote-region (point) (mark)))

Did you consider using point-max instead of mark?  IIRC, that mark was
very recently introduced which, perhaps irrationally, makes it seem
less future-proof to me.

>  
>  (defun notmuch-mua-forward-message ()
>    (message-forward)

Speaking of future-proofing, it would be good to have a test.
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Aaron Ecay]

On Thu, 19 Jan 2012 17:46:31 -0500, Austin Clements <[hidden email]> wrote:

> > ### OT:
> > For some reason, `mml-quote-region' explicitly re-quotes
> > already quoted MML tags:
> >
> >   "<#!*/?\\(multipart\\|part\\|external\\|mml\\)"
> >
> > Why is that ?
>
> Probably so the transformation is invertible, though as far as I can
> tell there's no mml-unquote-region.

Sending the message (or doing M-x mml-preview) undoes the quoting.  So
if the original message contains an already-quoted tag, constructing the
reply double-quotes it, and sending the reply will produce the original
single-quoted tag again.

--
Aaron Ecay
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Aaron Ecay]

On Thu, 19 Jan 2012 17:48:42 -0500, Austin Clements <[hidden email]> wrote:
> Did you consider using point-max instead of mark?  IIRC, that mark was
> very recently introduced which, perhaps irrationally, makes it seem
> less future-proof to me.

Well, if the patch goes in and someone changes the code so that it no
longer sets the mark (in the same way), they will be the one breaking
stuff, and they’ll have to come up with the fix themself.  Using point-max
would include the signature in the quoting as well.  It would probably be
fairly odd to want to put an MML tag in one’s signature, but that doesn’t
mean that we should break that usage.

>
> >  
> >  (defun notmuch-mua-forward-message ()
> >    (message-forward)
>
> Speaking of future-proofing, it would be good to have a test.

It would.  ;)  I’ll work on one.

--
Aaron Ecay
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Pieter Praet]

On Thu, 19 Jan 2012 17:52:23 -0500, Aaron Ecay <[hidden email]> wrote:

> On Thu, 19 Jan 2012 17:46:31 -0500, Austin Clements <[hidden email]> wrote:
> > > ### OT:
> > > For some reason, `mml-quote-region' explicitly re-quotes
> > > already quoted MML tags:
> > >
> > >   "<#!*/?\\(multipart\\|part\\|external\\|mml\\)"
> > >
> > > Why is that ?
> >
> > Probably so the transformation is invertible, though as far as I can
> > tell there's no mml-unquote-region.
>
> Sending the message (or doing M-x mml-preview) undoes the quoting.  So
> if the original message contains an already-quoted tag, constructing the
> reply double-quotes it, and sending the reply will produce the original
> single-quoted tag again.
>

Thanks!

This list just keeps on giving;  Free education, I tell ya...


> --
> Aaron Ecay


Peace

--
Pieter
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Pieter Praet]

On Thu, 19 Jan 2012 17:56:16 -0500, Aaron Ecay <[hidden email]> wrote:
> On Thu, 19 Jan 2012 17:48:42 -0500, Austin Clements <[hidden email]> wrote:
> > Did you consider using point-max instead of mark?  IIRC, that mark was
> > very recently introduced which, perhaps irrationally, makes it seem
> > less future-proof to me.
>
> Well, if the patch goes in and someone changes the code so that it no
> longer sets the mark (in the same way), they will be the one breaking
> stuff, and they’ll have to come up with the fix themself.  [...]

True that.


> [...] Using point-max
> would include the signature in the quoting as well.  It would probably be
> fairly odd to want to put an MML tag in one’s signature, but that doesn’t
> mean that we should break that usage.
>

So, would I be right to assume MML tags in signatures are never
evaluated to begin with?  Otherwise, there would still be a security
hole, no?


> >
> > >  
> > >  (defun notmuch-mua-forward-message ()
> > >    (message-forward)
> >
> > Speaking of future-proofing, it would be good to have a test.
>
> It would.  ;)  I’ll work on one.
>

Thanks!

These might save you some time:
  id:"[hidden email]"


> --
> Aaron Ecay
> _______________________________________________
> notmuch mailing list
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch


Peace

--
Pieter
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Aaron Ecay]

On Fri, 20 Jan 2012 00:21:08 +0100, Pieter Praet <[hidden email]> wrote:
> So, would I be right to assume MML tags in signatures are never
> evaluated to begin with?  Otherwise, there would still be a security
> hole, no?

I am thinking of MML tags that a user puts in their own signature.
If that case is a security hole, then the hole is in the user’s brain
and not in notmuch.  :)

--
Aaron Ecay
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[David Edmondson]

On Thu, 19 Jan 2012 13:43:09 -0500, Aaron Ecay <[hidden email]> wrote:
> -  (message-goto-body))
> +  (message-goto-body)
> +  (mml-quote-region (point) (mark)))

Obviously good. It would be nice to have a comment about why it's `mark'
and not `point-max'. In fact, it would be good to have a comment
explaining why `mml-quote-region' is required.

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[David Bremner-2]

On Thu, 19 Jan 2012 13:43:09 -0500, Aaron Ecay <[hidden email]> wrote:
> Emacs message-mode uses certain text strings to indicate how to attach
> files to outgoing mail.  If these are present in the text of an email,
> and a user is tricked into replying to the message, the user’s files
> could be exposed.
> ---

Can you include a NEWS patch against release with next version? We
should probably roll 0.11.1 with this fix.

d
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Pieter Praet]

On Thu, 19 Jan 2012 22:26:02 -0500, Aaron Ecay <[hidden email]> wrote:
> On Fri, 20 Jan 2012 00:21:08 +0100, Pieter Praet <[hidden email]> wrote:
> > So, would I be right to assume MML tags in signatures are never
> > evaluated to begin with?  Otherwise, there would still be a security
> > hole, no?
>
> I am thinking of MML tags that a user puts in their own signature.
> If that case is a security hole, then the hole is in the user’s brain
> and not in notmuch.  :)
>

Ah, right...  I didn't bother checking what the mark's position would be,
so assumed we were talking about the signature in the *quoted* message.

Won't happen again :)

> --
> Aaron Ecay


Peace

--
Pieter
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Austin Clements]

Quoth Aaron Ecay on Jan 19 at  5:56 pm:
> On Thu, 19 Jan 2012 17:48:42 -0500, Austin Clements <[hidden email]> wrote:
> > >  
> > >  (defun notmuch-mua-forward-message ()
> > >    (message-forward)
> >
> > Speaking of future-proofing, it would be good to have a test.
>
> It would.  ;)  I’ll work on one.

Were you planning to roll a new version of this patch with a test?
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Aaron Ecay]

The test is broken at this time; the next commit will introduce a fix.
---

Thanks for the reminder, Austin.  Things got hectic, and it took a
little bludgeoning to get the test suite to behave.  I *think* I got
it, although I am by no means confident.  Specifically, I am seeing
some unrelated(?) test failures in the emacs suite, so I'd appreciate
it if someone with a well-functioning test suite tried out these
patches before they are pushed, to be sure I didn't break anything.

I tried to follow the "first patch introduces a failing test, second
patch fixes it" convention.

 test/emacs                           |   14 ++++++++++++++
 test/emacs.expected-output/quote-mml |    9 +++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs.expected-output/quote-mml

diff --git a/test/emacs b/test/emacs
index 8ca4c8a..a57513a 100755
--- a/test/emacs
+++ b/test/emacs
@@ -273,6 +273,20 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <[hidden email]> w
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Quote MML tags on reply"
+test_subtest_known_broken
+add_message '[from]="1337 h4xor <[hidden email]>"' \
+            '[to]="Unsuspecting rube <[hidden email]>"' \
+            '[subject]="hackety hack hack"' \
+            '[body]="<#part type="application/octet-stream" filename="foo" disposition=attachment>
+<#/part>"' \
+            '[id]="[hidden email]"'
+test_emacs "(notmuch-show \"id:[hidden email]\")
+            (notmuch-show-reply-sender)
+            (test-output)"
+test_expect_equal_file OUTPUT "$EXPECTED/quote-mml"
+
+
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
 # save as archive to test that Emacs does not re-compress .gz
 test_emacs '(let ((standard-input "\"attachment1.gz\""))
diff --git a/test/emacs.expected-output/quote-mml b/test/emacs.expected-output/quote-mml
new file mode 100644
index 0000000..01bd2ca
--- /dev/null
+++ b/test/emacs.expected-output/quote-mml
@@ -0,0 +1,9 @@
+From: Notmuch Test Suite <[hidden email]>
+To: 1337 h4xor <[hidden email]>
+Subject: Re: hackety hack hack
+In-Reply-To: <[hidden email]>
+Fcc: /home/aecay/development/notmuch/test/tmp.emacs/mail/sent
+--text follows this line--
+On Fri, 05 Jan 2001 15:43:57 +0000, 1337 h4xor <[hidden email]> wrote:
+> <#!part type=application/octet-stream filename=foo disposition=attachment>
+> <#!/part>
--
1.7.9

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Aaron Ecay]

Emacs message-mode uses certain text strings to indicate how to attach
files to outgoing mail.  If these are present in the text of an email,
and a user is tricked into replying to the message, the user’s files
could be exposed.
---
 NEWS                 |   18 ++++++++++++++++++
 emacs/notmuch-mua.el |    3 ++-
 test/emacs           |    1 -
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 2acdce5..c8b90c7 100644
--- a/NEWS
+++ b/NEWS
@@ -56,6 +56,24 @@ Compatibility with GMime 2.6
   However, a bug in current GMime 2.6 causes notmuch not to report
   signatures where the signer key is unavailable (GNOME bug 668085).
 
+Notmuch 0.11.1 (2012-xx-xx)
+===========================
+
+Emacs Interface
+---------------
+
+Quote MML tags in replies
+
+  MML tags are text codes that Emacs uses to indicate attachments
+  (among other things) in messages being composed.  The Emacs
+  interface did not quote MML tags in the quoted text of a reply.  If
+  a user could be tricked into replying to a maliciously formatted
+  message and not editing out the MML tags from the quoted text, this
+  could lead to files from the user's machine being attached to the
+  outgoing message.  The Emacs interface now quotes these tags in
+  reply text, so that they cannot have an effect on the outgoing
+  message.
+
 Notmuch 0.11 (2012-01-13)
 =========================
 
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 023645e..32c376d 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -116,7 +116,8 @@ list."
     (push-mark))
   (set-buffer-modified-p nil)
 
-  (message-goto-body))
+  (message-goto-body)
+  (mml-quote-region (point) (mark)))
 
 (defun notmuch-mua-forward-message ()
   (message-forward)
diff --git a/test/emacs b/test/emacs
index a57513a..affcca4 100755
--- a/test/emacs
+++ b/test/emacs
@@ -274,7 +274,6 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Quote MML tags on reply"
-test_subtest_known_broken
 add_message '[from]="1337 h4xor <[hidden email]>"' \
             '[to]="Unsuspecting rube <[hidden email]>"' \
             '[subject]="hackety hack hack"' \
--
1.7.9

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Tomi Ollila-2]

On Sun, 29 Jan 2012 01:07:08 -0500, Aaron Ecay <[hidden email]> wrote:
> Emacs message-mode uses certain text strings to indicate how to attach
> files to outgoing mail.  If these are present in the text of an email,
> and a user is tricked into replying to the message, the user’s files
> could be exposed.
> ---

[ ... ]

>  
> -  (message-goto-body))
> +  (message-goto-body)
> +  (mml-quote-region (point) (mark)))

As asked before, comment why mmm-quote-region is needed
and why (mark) is used instead of (point-max) is there.
We know the reasons but future viewers of the code will
wonder a while...

Tomi
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[David Bremner-2]

On Sun, 29 Jan 2012 01:07:07 -0500, Aaron Ecay <[hidden email]> wrote:
> The test is broken at this time; the next commit will introduce a fix.
> ---

Hi Aaron.

Applied to master test fails for me as follows (after the second patch
is applied as well).

         In-Reply-To: <[hidden email]>
        -Fcc: /home/aecay/development/notmuch/test/tmp.emacs/mail/sent
        +Fcc: /home/bremner/software/upstream/notmuch/test/tmp.emacs/mail/sent

I guess it should force Fcc somehow in the test; didn't have time to dig
further.

Do you mind making another round, against the release branch this time
and taking the commit message comments into account?

d
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Dmitry Kurochkin]

Hi Aaron.

Thanks for your work!  I took the liberty to do some cleanups for your
patch.  Below is a detailed list of changes.

Hope this helps.

Changes since v2:

* change patch names to be consistent with others:

  - s/emacs:/test:/ for the test patch

  - lower case the first word after colon in the patch title

* polish NEWS wording, move it to 0.12 section

* add comment to `mml-quote-region' call, as suggested by Tomi [1]

* fix and clean up the test:

  - set `notmuch-fcc-dirs' to nil to avoid adding the Fcc header,
    otherwise it breaks the test on other systems as pointed by
    David [2]

  - use default values for add_message parameters where possible

  - use a sane subject value in add_message

  - use shorter MML tag as produced by (mml-insert-part)

  - indenting and other minor cleanups

Regards,
  Dmitry

[1] id:"[hidden email]"
[2] id:"[hidden email]"

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Dmitry Kurochkin]

From: Aaron Ecay <[hidden email]>

The test is broken at this time; the next commit will introduce a fix.
---
 test/emacs                                    |   12 ++++++++++++
 test/emacs.expected-output/quote-mml-in-reply |    7 +++++++
 2 files changed, 19 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs.expected-output/quote-mml-in-reply

diff --git a/test/emacs b/test/emacs
index 8ca4c8a..a3f4893 100755
--- a/test/emacs
+++ b/test/emacs
@@ -273,6 +273,18 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <[hidden email]> w
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Quote MML tags in reply"
+test_subtest_known_broken
+message_id='[hidden email]'
+add_message [id]="$message_id" \
+    "[subject]='$test_subtest_name'" \
+    '[body]="<#part disposition=inline>"'
+test_emacs "(let ((notmuch-fcc-dirs nil))
+      (notmuch-show \"id:$message_id\")
+      (notmuch-show-reply)
+      (test-output))"
+test_expect_equal_file OUTPUT "$EXPECTED/quote-mml-in-reply"
+
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
 # save as archive to test that Emacs does not re-compress .gz
 test_emacs '(let ((standard-input "\"attachment1.gz\""))
diff --git a/test/emacs.expected-output/quote-mml-in-reply b/test/emacs.expected-output/quote-mml-in-reply
new file mode 100644
index 0000000..adec92a
--- /dev/null
+++ b/test/emacs.expected-output/quote-mml-in-reply
@@ -0,0 +1,7 @@
+From: Notmuch Test Suite <[hidden email]>
+To:
+Subject: Re: Quote MML tags in reply
+In-Reply-To: <[hidden email]>
+--text follows this line--
+On Fri, 05 Jan 2001 15:43:57 +0000, Notmuch Test Suite <[hidden email]> wrote:
+> <#!part disposition=inline>
--
1.7.9

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Dmitry Kurochkin]

From: Aaron Ecay <[hidden email]>

Emacs message-mode uses certain text strings to indicate how to attach
files to outgoing mail.  If these are present in the text of an email,
and a user is tricked into replying to the message, the user’s files
could be exposed.
---
 NEWS                 |   12 ++++++++++++
 emacs/notmuch-mua.el |    7 ++++++-
 test/emacs           |    1 -
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 2acdce5..ef26b8c 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,17 @@ Reply to sender
   and search modes, 'r' has been bound to reply to sender, replacing
   reply to all, which now has key binding 'R'.
 
+Quote MML tags in replies
+
+  MML tags are text codes that Emacs uses to indicate attachments
+  (among other things) in messages being composed.  The Emacs
+  interface did not quote MML tags in the quoted text of a reply.
+  User could be tricked into replying to a maliciously formatted
+  message and not editing out the MML tags from the quoted text.  This
+  could lead to files from the user's machine being attached to the
+  outgoing message.  The Emacs interface now quotes these tags in
+  reply text, so that they do not effect outgoing messages.
+
 Library changes
 ---------------
 
@@ -56,6 +67,7 @@ Compatibility with GMime 2.6
   However, a bug in current GMime 2.6 causes notmuch not to report
   signatures where the signer key is unavailable (GNOME bug 668085).
 
+
 Notmuch 0.11 (2012-01-13)
 =========================
 
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 023645e..4be7c13 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -116,7 +116,12 @@ list."
     (push-mark))
   (set-buffer-modified-p nil)
 
-  (message-goto-body))
+  (message-goto-body)
+  ;; Original message may contain (malicious) MML tags.  We must
+  ;; properly quote them in the reply.  Note that using `point-max'
+  ;; instead of `mark' here is wrong.  The buffer may include user's
+  ;; signature which should not be MML-quoted.
+  (mml-quote-region (point) (mark)))
 
 (defun notmuch-mua-forward-message ()
   (message-forward)
diff --git a/test/emacs b/test/emacs
index a3f4893..b9f7d15 100755
--- a/test/emacs
+++ b/test/emacs
@@ -274,7 +274,6 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Quote MML tags in reply"
-test_subtest_known_broken
 message_id='[hidden email]'
 add_message [id]="$message_id" \
     "[subject]='$test_subtest_name'" \
--
1.7.9

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch