GNU bug report logs - #28350
CVE-2017-14482: enriched.el code execution

Previous Next

Package: emacs;

Reported by: charles <at> aurox.ch (Charles A. Roelli)

Date: Mon, 4 Sep 2017 19:26:01 UTC

Severity: important

Tags: security

Found in versions 25.1, 23.1, 21.4, 23.2, 21.2, 22.3, 24.3, 21.1, 21.3, 24.1, 24.5, 25.2, 24.2, 23.4, 22.1, 23.3, 24.4, 22.2

Fixed in version 25.3

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 28350 in the body.
You can then email your comments to 28350 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 04 Sep 2017 19:26:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to charles <at> aurox.ch (Charles A. Roelli):
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 04 Sep 2017 19:26:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: bug-gnu-emacs <at> gnu.org
Subject: enriched.el code execution
Date: Mon, 04 Sep 2017 21:24:42 +0200
[Message part 1 (text/plain, inline)]
Enriched mode implements an extension command to the text/enriched
format called "x-display", which stores "display" text properties.
It was added awhile ago:

  commit d9e28c1ca1d95f51a05d052dcf1fe06888d52476
  Author: Gerd Moellmann <gerd <at> gnu.org>
  Date:   Wed Jul 21 21:43:03 1999 +0000

  (enriched-translations): Add `display' and "x-display".
  (enriched-handle-display-prop): New.
  (enriched-decode-display-prop): New.

It's possible to use this extension command to transparently execute
arbitrary code in an Emacs process that opens a text/enriched file.
For example, if you open a file containing the following contents:

Content-Type: text/enriched
Text-Width: 70

<x-display><param>(when (message "hello world") nil)</param>test</x-display>

Then "hello world" will be printed in the echo area whenever the
"test" text is displayed (which is immediate).  Note that the
s-expression between the <param> tags needs to conform to a "display"
spec: but since there are a few display specs that can execute code,
it's not difficult to craft a file that could have bad effects (shell
commands work, for example).  Additionally, such a file can be
compressed with gzip (thus hiding the contents), and when it is
opened, Emacs will automatically decompress it and apply the display
properties.  Attached is an example file (enriched-bug-example.txt)
that turns the mode line red as soon as you open it.  It works in
23.4, 24.5, 25.2 and master (and possibly earlier versions -- I
haven't tested).

Other extensions in `enriched-translations' of enriched.el may have
similar issues (I don't understand them all, so I hope somebody else
can make sure).

[enriched-bug-example.txt (text/enriched, attachment)]

Severity set to 'important' from 'normal' Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Tue, 05 Sep 2017 15:36:01 GMT) Full text and rfc822 format available.

Added tag(s) security. Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Tue, 05 Sep 2017 15:36:01 GMT) Full text and rfc822 format available.

Added indication that bug 28350 blocks24655 Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Tue, 05 Sep 2017 15:36:02 GMT) Full text and rfc822 format available.

Removed indication that bug 28350 blocks Request was from Eli Zaretskii <eliz <at> gnu.org> to control <at> debbugs.gnu.org. (Tue, 05 Sep 2017 18:55:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Wed, 06 Sep 2017 19:26:02 GMT) Full text and rfc822 format available.

Message #16 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Wed, 06 Sep 2017 21:25:18 +0200
If anyone wants a fix to apply locally, the following s-expression
prevents the display parameter from being used by Enriched mode
(tested in Emacs 23+):

(eval-after-load "enriched"
  '(defun enriched-decode-display-prop (start end &optional param)
     (list start end)))

As for a fix to apply to master: I'd like to keep "x-display" if we
can agree on some "safe" predicate that the given parameter would have
to satisfy.  Looking at the list of display specifications that are
available, it seems that simple string, margin text, space-width,
height (only in the (+ n), (- n) and n cases) and raise specifications
should be okay.  Does anybody else have an opinion about this?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Thu, 07 Sep 2017 02:35:02 GMT) Full text and rfc822 format available.

Message #19 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Thu, 07 Sep 2017 05:34:34 +0300
> Date: Wed, 06 Sep 2017 21:25:18 +0200
> From: charles <at> aurox.ch (Charles A. Roelli)
> 
> As for a fix to apply to master: I'd like to keep "x-display" if we
> can agree on some "safe" predicate that the given parameter would have
> to satisfy.  Looking at the list of display specifications that are
> available, it seems that simple string, margin text, space-width,
> height (only in the (+ n), (- n) and n cases) and raise specifications
> should be okay.  Does anybody else have an opinion about this?

I agree that the cases you have shown are safe.

Thanks.




bug Marked as found in versions 21.1, 21.4, 23.2, 24.3, 24.2, 25.2, 22.1, 24.5, 24.1, 22.2, 23.4, 23.1, 24.4, 23.3, 21.2, 21.3, 22.3, and 25.1. Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Fri, 08 Sep 2017 20:23:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Sat, 09 Sep 2017 12:25:01 GMT) Full text and rfc822 format available.

Message #24 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Sat, 09 Sep 2017 14:23:54 +0200
[Message part 1 (text/plain, inline)]
> Date: Thu, 07 Sep 2017 05:34:34 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> CC: 28350 <at> debbugs.gnu.org
> 
> > Date: Wed, 06 Sep 2017 21:25:18 +0200
> > From: charles <at> aurox.ch (Charles A. Roelli)
> > 
> > As for a fix to apply to master: I'd like to keep "x-display" if we
> > can agree on some "safe" predicate that the given parameter would have
> > to satisfy.  Looking at the list of display specifications that are
> > available, it seems that simple string, margin text, space-width,
> > height (only in the (+ n), (- n) and n cases) and raise specifications
> > should be okay.  Does anybody else have an opinion about this?
> 
> I agree that the cases you have shown are safe.
> 
> Thanks.

Thank you.  Does the attached patch look OK?  I've used the file
enriched-test-safe-props.txt (also attached) to test that safe
properties are still applied.

[0001-Prevent-code-execution-by-text-enriched-files-Bug-28.patch (text/x-patch, attachment)]
[enriched-test-safe-props.txt (text/enriched, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Sat, 09 Sep 2017 13:46:01 GMT) Full text and rfc822 format available.

Message #27 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Sat, 09 Sep 2017 16:45:40 +0300
> Date: Sat, 09 Sep 2017 14:23:54 +0200
> From: charles <at> aurox.ch (Charles A. Roelli)
> CC: 28350 <at> debbugs.gnu.org
> 
> Thank you.  Does the attached patch look OK?  I've used the file
> enriched-test-safe-props.txt (also attached) to test that safe
> properties are still applied.

Thank you for working on this.  I have some comments:

> --- a/lisp/textmodes/enriched.el
> +++ b/lisp/textmodes/enriched.el
> @@ -503,6 +503,47 @@ enriched-decode-display-prop
>  		  (error nil)))))
>      (unless prop
>        (message "Warning: invalid <x-display> parameter %s" param))
> -    (list start end 'display prop)))
> +    (if (enriched-display-prop-safe-p prop)
> +        (list start end 'display prop)
> +      (message "Warning: unsafe <x-display> parameter %s not applied" param)
> +      (list start end))))

I think we will want to allow unsafe display properties, given a
user's explicit permission.  So I think we need a defcustom that
allows this, and then enriched-display-prop-safe-p should always
return non-nil.

> +See Info node `(elisp)Display Property' for the use of these
> +display specifications."
> +  (ignore-errors
> +    (or (stringp prop)
            ^^^^^^^^^^^^
What about an image spec (including a slice spec)?

> +        (and (eq (car prop) 'space-width)
> +             (or (integerp (cadr prop)) (floatp (cadr prop))))
> +        (and (consp (car prop))
> +             (eq (caar prop) 'margin)
> +             (or (eq (cadar prop) 'right-margin)
> +                 (eq (cadar prop) 'left-margin))
> +             (stringp (cadr prop)))

The margin display can also specify an image, not just a string, and I
think that would be safe as well.

> +        (and (eq (car prop) 'height)
> +             (or (integerp (cadr prop))
> +                 (and (listp (cadr prop))
> +                      (or (eq (elt (cadr prop) 0) '+) (elt (cadr prop) 0) '-)
> +                      (integerp (elt (cadr prop) 1)))))
                          ^^^^^^^^
I think this should be numberp, as the value could also safely be a
float.

> +        (and (eq (car prop) 'raise)
> +             (integerp (cadr prop))))))
                 ^^^^^^^^
The FACTOR in (raise FACTOR) can also be a float, so I think numberp
is the correct predicate here.

And then what about (space . PROPS) type of display spec?  I think all
of its variants are safe.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Sat, 09 Sep 2017 15:58:02 GMT) Full text and rfc822 format available.

Message #30 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Sat, 09 Sep 2017 17:57:10 +0200
Thanks for the feedback.

> Date: Sat, 09 Sep 2017 16:45:40 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> CC: 28350 <at> debbugs.gnu.org
> Reply-to: Eli Zaretskii <eliz <at> gnu.org>
> 
> > --- a/lisp/textmodes/enriched.el
> > +++ b/lisp/textmodes/enriched.el
> > @@ -503,6 +503,47 @@ enriched-decode-display-prop
> >  		  (error nil)))))
> >      (unless prop
> >        (message "Warning: invalid <x-display> parameter %s" param))
> > -    (list start end 'display prop)))
> > +    (if (enriched-display-prop-safe-p prop)
> > +        (list start end 'display prop)
> > +      (message "Warning: unsafe <x-display> parameter %s not applied" param)
> > +      (list start end))))
> 
> I think we will want to allow unsafe display properties, given a
> user's explicit permission.  So I think we need a defcustom that
> allows this, and then enriched-display-prop-safe-p should always
> return non-nil.

Agreed, I've added this.

> > +See Info node `(elisp)Display Property' for the use of these
> > +display specifications."
> > +  (ignore-errors
> > +    (or (stringp prop)
>             ^^^^^^^^^^^^
> What about an image spec (including a slice spec)?

Okay, I see that image specs can be safe.  But are they all safe?

And I don't understand how a slice spec is used together with an image
spec.  Is the slice spec used inside of IMAGE-PROPS, i.e. as you might
gather from the manual:

‘(image . IMAGE-PROPS)’
     This kind of display specification is an image descriptor (*note
     Images).  When used as a display specification, it means to
     display the image instead of the text that has the display
     specification.

‘(slice X Y WIDTH HEIGHT)’
     This specification together with ‘image’ specifies a “slice” (a
     partial area) of the image to display. 

?

> 
> > +        (and (eq (car prop) 'space-width)
> > +             (or (integerp (cadr prop)) (floatp (cadr prop))))
> > +        (and (consp (car prop))
> > +             (eq (caar prop) 'margin)
> > +             (or (eq (cadar prop) 'right-margin)
> > +                 (eq (cadar prop) 'left-margin))
> > +             (stringp (cadr prop)))
> 
> The margin display can also specify an image, not just a string, and I
> think that would be safe as well.

Okay, I'll apply the same procedure as we decide for the above image
spec.

> 
> > +        (and (eq (car prop) 'height)
> > +             (or (integerp (cadr prop))
> > +                 (and (listp (cadr prop))
> > +                      (or (eq (elt (cadr prop) 0) '+) (elt (cadr prop) 0) '-)
> > +                      (integerp (elt (cadr prop) 1)))))
>                           ^^^^^^^^
> I think this should be numberp, as the value could also safely be a
> float.
> 
> > +        (and (eq (car prop) 'raise)
> > +             (integerp (cadr prop))))))
>                  ^^^^^^^^
> The FACTOR in (raise FACTOR) can also be a float, so I think numberp
> is the correct predicate here.
> 
> And then what about (space . PROPS) type of display spec?  I think all
> of its variants are safe.

Okay, I've made these changes and added the `space' spec.  At this
point it seems that unsafe display specs are more the exception than
the rule, so it might make sense to define the
`enriched-display-prop-safe-p' function by excluding the unsafe
specifications instead of including the safe ones.  What do you think?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Sat, 09 Sep 2017 16:57:02 GMT) Full text and rfc822 format available.

Message #33 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Sat, 09 Sep 2017 19:55:37 +0300
> Date: Sat, 09 Sep 2017 17:57:10 +0200
> From: charles <at> aurox.ch (Charles A. Roelli)
> CC: 28350 <at> debbugs.gnu.org
> 
> > > +See Info node `(elisp)Display Property' for the use of these
> > > +display specifications."
> > > +  (ignore-errors
> > > +    (or (stringp prop)
> >             ^^^^^^^^^^^^
> > What about an image spec (including a slice spec)?
> 
> Okay, I see that image specs can be safe.  But are they all safe?

I think they are.  Does anyone know different?

> And I don't understand how a slice spec is used together with an image
> spec.  Is the slice spec used inside of IMAGE-PROPS, i.e. as you might
> gather from the manual:
> 
> ‘(image . IMAGE-PROPS)’
>      This kind of display specification is an image descriptor (*note
>      Images).  When used as a display specification, it means to
>      display the image instead of the text that has the display
>      specification.
> 
> ‘(slice X Y WIDTH HEIGHT)’
>      This specification together with ‘image’ specifies a “slice” (a
>      partial area) of the image to display. 
> 
> ?

AFAIU, like this:

  ((slice X Y WIDTH HEIGHT) (image . IMAGE-PROPS))

You can see examples of this in image.el and image-mode.el.

> At this point it seems that unsafe display specs are more the
> exception than the rule, so it might make sense to define the
> `enriched-display-prop-safe-p' function by excluding the unsafe
> specifications instead of including the safe ones.  What do you
> think?

I'm not sure.  The display spec can be complex, so to make sure none
of these exceptions sneak through, you will have to recursively unpack
the spec data structure and examine each of the elements, which smells
too similar to emulating 'eval'.  No?

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Sat, 09 Sep 2017 20:39:02 GMT) Full text and rfc822 format available.

Message #36 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Sat, 09 Sep 2017 22:37:29 +0200
[Message part 1 (text/plain, inline)]
> Date: Sat, 09 Sep 2017 19:55:37 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> CC: 28350 <at> debbugs.gnu.org
>
> > > > +See Info node `(elisp)Display Property' for the use of these
> > > > +display specifications."
> > > > +  (ignore-errors
> > > > +    (or (stringp prop)
> > >             ^^^^^^^^^^^^
> > > What about an image spec (including a slice spec)?
> > 
> > Okay, I see that image specs can be safe.  But are they all safe?
> 
> I think they are.  Does anyone know different?

I read over the documentation some more and they do look alright.

> > And I don't understand how a slice spec is used together with an image
> > spec.  Is the slice spec used inside of IMAGE-PROPS, i.e. as you might
> > gather from the manual:
> > 
> > ‘(image . IMAGE-PROPS)’
> >      This kind of display specification is an image descriptor (*note
> >      Images).  When used as a display specification, it means to
> >      display the image instead of the text that has the display
> >      specification.
> > 
> > ‘(slice X Y WIDTH HEIGHT)’
> >      This specification together with ‘image’ specifies a “slice” (a
> >      partial area) of the image to display. 
> > 
> > ?
> 
> AFAIU, like this:
> 
>   ((slice X Y WIDTH HEIGHT) (image . IMAGE-PROPS))
> 
> You can see examples of this in image.el and image-mode.el.

Thanks.  I forgot that the display property can be set to a list or
vector of display specifications.  I've updated the patch to reflect
this:

+        (and (seqp prop) (seq-every-p 'enriched-display-prop-safe-p prop)))))

and I've added slice/image specifications.

> > At this point it seems that unsafe display specs are more the
> > exception than the rule, so it might make sense to define the
> > `enriched-display-prop-safe-p' function by excluding the unsafe
> > specifications instead of including the safe ones.  What do you
> > think?
> 
> I'm not sure.  The display spec can be complex, so to make sure none
> of these exceptions sneak through, you will have to recursively unpack
> the spec data structure and examine each of the elements, which smells
> too similar to emulating 'eval'.  No?

Thank you.  I've kept the current approach.  Please see again the
attached patch.

Also, should the left-fringe/right-fringe display specifications be
considered safe?  They seem innocuous.

[0001-Prevent-code-execution-by-text-enriched-files-Bug-28.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Sat, 09 Sep 2017 22:44:01 GMT) Full text and rfc822 format available.

Message #39 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: "Charles A. Roelli" <charles <at> aurox.ch>
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: enriched.el code execution
Date: Sat, 9 Sep 2017 15:43:30 -0700
Thanks for reporting this bug. Since it is a serious security hole I have 
installed a patch by Lars Ingebrigtsen that temporarily disables the problematic 
translations, and that also changes Gnus to not call enriched-decode. For the 
emacs-25 branch the patch is here:

https://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-25&id=9ad0fcc54442a9a01d41be19880250783426db70

and for the master branch the patch is here:

https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=19584f13b1e2e4a778602a8302619ef5c675e68b

As this patch is merely a workaround to close the security hole, I am not 
marking the underlying bug as fixed.

Thank you for reporting the problem.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Sun, 10 Sep 2017 17:02:01 GMT) Full text and rfc822 format available.

Message #42 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Sun, 10 Sep 2017 20:01:20 +0300
> Date: Sat, 09 Sep 2017 22:37:29 +0200
> From: charles <at> aurox.ch (Charles A. Roelli)
> CC: 28350 <at> debbugs.gnu.org
> 
> Thank you.  I've kept the current approach.  Please see again the
> attached patch.

Some minor nits below.

> Also, should the left-fringe/right-fringe display specifications be
> considered safe?  They seem innocuous.

Yes, I think so.  And your patch already does allow them, doesn't it?

> +(defcustom enriched-allow-unsafe-display-props nil
> +  "Variable determining whether to decode arbitrary display properties.

"If non-nil allow to evaluate arbitrary forms in display properties."

> +  :risky t
> +  :type 'boolean
> +  :group 'enriched)

Please add :version here.  Please also add a short NEWS entry.

It would be good to have tests for this, but doing that is much less
urgent than fixing the vulnerability, so please feel free to do so as
a separate commit (unless you already have the tests ready).

Otherwise, looks good to me.  Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Sun, 10 Sep 2017 18:55:01 GMT) Full text and rfc822 format available.

Message #45 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: larsi <at> gnus.org, 28350 <at> debbugs.gnu.org
Subject: Re: enriched.el code execution
Date: Sun, 10 Sep 2017 20:54:13 +0200
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Sat, 9 Sep 2017 15:43:30 -0700
> 
> Thanks for reporting this bug. Since it is a serious security hole I have 
> installed a patch by Lars Ingebrigtsen that temporarily disables the problematic 
> translations, and that also changes Gnus to not call enriched-decode. For the 
> emacs-25 branch the patch is here:
> 
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-25&id=9ad0fcc54442a9a01d41be19880250783426db70
> 
> and for the master branch the patch is here:
> 
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=19584f13b1e2e4a778602a8302619ef5c675e68b
> 
> As this patch is merely a workaround to close the security hole, I am not 
> marking the underlying bug as fixed.
> 
> Thank you for reporting the problem.

Thanks for these fixes.  I have some comments:

> branch: master
> commit 19584f13b1e2e4a778602a8302619ef5c675e68b
> Author: Lars Ingebrigtsen <larsi <at> gnus.org>
> Commit: Paul Eggert <eggert <at> cs.ucla.edu>
>
> [...]
>
> --- a/lisp/textmodes/enriched.el
> +++ b/lisp/textmodes/enriched.el
> @@ -117,12 +117,7 @@ expression, which is evaluated to get the string to insert.")
>  		   (full        "flushboth")
>  		   (center      "center"))
>      (PARAMETER     (t           "param")) ; Argument of preceding annotation
> -    ;; The following are not part of the standard:
> -    (FUNCTION      (enriched-decode-foreground "x-color")
> -		   (enriched-decode-background "x-bg-color")

Do we know that "x-color" and/or "x-bg-color" are vulnerable to a
similar misuse as "x-display"?  If not, I can still re-add them at a
later time.

> branch: emacs-25
> commit b6389930146882a77c22901a4357e287826fc7ff
> Author: Paul Eggert <eggert <at> cs.ucla.edu>
> Commit: Paul Eggert <eggert <at> cs.ucla.edu>
>
> [...]
>
> +** Enriched text mode no longer supports the 'FUNCTION' and 'display'
> +translations, and Gnus no longer processes enriched text when
> +inlining.  This fixes bugs introduced in Emacs 19.29.  To work around
> +these bugs in Emacs versions 19.29 through 25.2, append the following
> +to your ~/.emacs file:
> +
> +  (provide 'enriched)
> +  (defun enriched-mode (&optional arg))
> +  (defun enriched-decode (from to))

This fix is very safe, at the cost of disabling Enriched mode.  Could
we do any better?  I had suggested the following (in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28350#16):

  (eval-after-load "enriched"
    '(defun enriched-decode-display-prop (start end &optional param)
       (list start end)))

But it may not work in Emacs earlier than 23 (I can't test it).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Sun, 10 Sep 2017 21:48:02 GMT) Full text and rfc822 format available.

Message #48 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: "Charles A. Roelli" <charles <at> aurox.ch>
Cc: larsi <at> gnus.org, 28350 <at> debbugs.gnu.org
Subject: Re: enriched.el code execution
Date: Sun, 10 Sep 2017 14:46:59 -0700
Charles A. Roelli wrote:

> Do we know that "x-color" and/or "x-bg-color" are vulnerable to a
> similar misuse as "x-display"?  If not, I can still re-add them at a
> later time.

Eli asked the same question privately. I don't know the code myself; perhaps 
Lars could say.

>> +  (provide 'enriched)
>> +  (defun enriched-mode (&optional arg))
>> +  (defun enriched-decode (from to))
> 
> This fix is very safe, at the cost of disabling Enriched mode.  Could
> we do any better?  I had suggested the following (in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28350#16):
> 
>    (eval-after-load "enriched"
>      '(defun enriched-decode-display-prop (start end &optional param)
>         (list start end)))
> 
> But it may not work in Emacs earlier than 23 (I can't test it).

It should work, since eval-after-load predates Emacs 19.29.  Though it assumes 
that x-display is the only problem here.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 02:40:01 GMT) Full text and rfc822 format available.

Message #51 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: larsi <at> gnus.org, charles <at> aurox.ch, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 05:39:27 +0300
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Sun, 10 Sep 2017 14:46:59 -0700
> Cc: larsi <at> gnus.org, 28350 <at> debbugs.gnu.org
> 
> >    (eval-after-load "enriched"
> >      '(defun enriched-decode-display-prop (start end &optional param)
> >         (list start end)))
> > 
> > But it may not work in Emacs earlier than 23 (I can't test it).
> 
> It should work, since eval-after-load predates Emacs 19.29.  Though it assumes 
> that x-display is the only problem here.

x-display _is_ the only problem, because only it allows arbitrary Lisp
forms.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 14:24:01 GMT) Full text and rfc822 format available.

Message #54 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: eggert <at> cs.ucla.edu
Cc: larsi <at> gnus.org, charles <at> aurox.ch, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 17:22:48 +0300
> Date: Mon, 11 Sep 2017 05:39:27 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: larsi <at> gnus.org, charles <at> aurox.ch, 28350 <at> debbugs.gnu.org
> 
> > From: Paul Eggert <eggert <at> cs.ucla.edu>
> > Date: Sun, 10 Sep 2017 14:46:59 -0700
> > Cc: larsi <at> gnus.org, 28350 <at> debbugs.gnu.org
> > 
> > >    (eval-after-load "enriched"
> > >      '(defun enriched-decode-display-prop (start end &optional param)
> > >         (list start end)))
> > > 
> > > But it may not work in Emacs earlier than 23 (I can't test it).
> > 
> > It should work, since eval-after-load predates Emacs 19.29.  Though it assumes 
> > that x-display is the only problem here.
> 
> x-display _is_ the only problem, because only it allows arbitrary Lisp
> forms.

I eventually decided to provide a simpler patch, see below.  The
original changes unnecessarily removed the capability to encode
display properties while saving Enriched Mode text, something that
doesn't have any security issues (because the vulnerability is on the
receiving end).  I also prefer not to remove the offending code, but
instead to comment it out, as I believe this is more in the tradition
of Free Software to let people eyeball what we did.  Finally, I
rewrote the NEWS entry to be more accurate wrt the actual change.

Nicolas is working on the release as we speak, so if someone has
suggestions, or objections, or something else important to say about
the patch, please speak up.

I'd like to take this opportunity to thank all those who worked and
continue working on fixing this vulnerability.


2017-09-11  Eli Zaretskii  <eliz <at> gnu.org>

	* etc/NEWS: Document the vulnerability and its resolution.
	Include a workaround.  Suggested by Charles A. Roelli
	<charles <at> aurox.ch>.

	* lisp/gnus/mm-view.el (mm-inline-text): Disable decoding of
	"enriched" and "richtext" MIME objects.  Suggested by Lars
	Ingebrigtsen <larsi <at> gnus.org>.

	* lisp/textmodes/enriched.el (enriched-decode-display-prop): Don't
	produce 'display' properties.  (Bug#28350)


--- lisp/textmodes/enriched.el~0	2017-02-03 12:25:44.000000000 +0200
+++ lisp/textmodes/enriched.el	2017-09-11 17:31:35.943569900 +0300
@@ -503,6 +503,9 @@
 		  (error nil)))))
     (unless prop
       (message "Warning: invalid <x-display> parameter %s" param))
-    (list start end 'display prop)))
+    ;; Disabled in Emacs 25.3 to avoid execution of arbitrary Lisp
+    ;; forms in display properties stored within enriched text.
+    ;; (list start end 'display prop)))
+    (list start end)))
 
 ;;; enriched.el ends here


--- lisp/gnus/mm-view.el~0	2017-02-03 12:25:44.000000000 +0200
+++ lisp/gnus/mm-view.el	2017-09-11 16:56:58.804519400 +0300
@@ -383,10 +383,12 @@
 	(goto-char (point-max))))
     (save-restriction
       (narrow-to-region b (point))
-      (when (member type '("enriched" "richtext"))
-        (set-text-properties (point-min) (point-max) nil)
-	(ignore-errors
-	  (enriched-decode (point-min) (point-max))))
+      ;; Disabled in Emacs 25.3 to avoid execution of arbitrary Lisp
+      ;; forms in display properties supported by enriched.el.
+      ;; (when (member type '("enriched" "richtext"))
+      ;;   (set-text-properties (point-min) (point-max) nil)
+      ;; 	(ignore-errors
+      ;; 	  (enriched-decode (point-min) (point-max))))
       (mm-handle-set-undisplayer
        handle
        `(lambda ()


--- etc/NEWS~0	2017-02-21 11:08:27.000000000 +0200
+++ etc/NEWS	2017-09-11 17:21:06.994252400 +0300
@@ -16,6 +16,32 @@
 with a prefix argument or by typing C-u C-h C-n.
 
 
+* Changes in Emacs 25.3
+
+This is an emergency release to fix a security vulnerability in Emacs.
+
+** Security vulnerability related to Enriched Text mode is removed.
+
+*** Enriched Text mode has its support for decoding 'x-display' disabled.
+This feature allows saving 'display' properties as part of text.
+Emacs 'display' properties support evaluation of arbitrary Lisp forms
+as part of instantiating the property, so decoding 'x-display' is
+vulnerable to executing arbitrary malicious Lisp code included in the
+text (e.g., sent as part of an email message).
+
+This vulnerability was introduced in Emacs 19.29.  To work around that
+in Emacs versions before 25.3, append the following to your ~/.emacs
+init file:
+
+  (eval-after-load "enriched"
+    '(defun enriched-decode-display-prop (start end &optional param)
+       (list start end)))
+
+*** Gnus no longer supports "richtext" and "enriched" inline MIME objects.
+This support was disabled to avoid evaluation of arbitrary Lisp code
+contained in email messages and news articles.
+
+
 * Changes in Emacs 25.2
 
 This is mainly a bug-fix release, but there are some other changes.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 15:19:02 GMT) Full text and rfc822 format available.

Message #57 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: larsi <at> gnus.org, eggert <at> cs.ucla.edu, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 18:18:01 +0300
> Date: Sun, 10 Sep 2017 20:54:13 +0200
> From: charles <at> aurox.ch (Charles A. Roelli)
> Cc: larsi <at> gnus.org, 28350 <at> debbugs.gnu.org
> 
> > --- a/lisp/textmodes/enriched.el
> > +++ b/lisp/textmodes/enriched.el
> > @@ -117,12 +117,7 @@ expression, which is evaluated to get the string to insert.")
> >  		   (full        "flushboth")
> >  		   (center      "center"))
> >      (PARAMETER     (t           "param")) ; Argument of preceding annotation
> > -    ;; The following are not part of the standard:
> > -    (FUNCTION      (enriched-decode-foreground "x-color")
> > -		   (enriched-decode-background "x-bg-color")
> 
> Do we know that "x-color" and/or "x-bg-color" are vulnerable to a
> similar misuse as "x-display"?

They are not.  They are converted to face properties, whose values
don't support Lisp evaluation.

> > +  (provide 'enriched)
> > +  (defun enriched-mode (&optional arg))
> > +  (defun enriched-decode (from to))
> 
> This fix is very safe, at the cost of disabling Enriched mode.  Could
> we do any better?  I had suggested the following (in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28350#16):
> 
>   (eval-after-load "enriched"
>     '(defun enriched-decode-display-prop (start end &optional param)
>        (list start end)))

You are right, and I therefore asked Nicolas to put that as a
workaround into NEWS of Emacs 25.3.

Anyway, I think I have a better idea for how to fix this on master.
And I'm very sorry that this idea didn't come to me earlier, before
you invested all these efforts in your patch.  (I'm also surprised
that no one here had beaten me up to this idea.)

Here's the idea: we introduce a new form of a display property:

  ('disable-eval SPEC)

where SPEC is anything supported in a display property.  Then we
change the implementation of display property in xdisp.c such that
when the above form is seen, we disable Lisp evaluation while walking
SPEC and producing display from it in the display engine.  Such a
patch to xdisp.c appears below.  Then enriched.el will have to either
add disable-eval wrapper around any display property, or not add it if
the user customized enriched.el to enable such evaluation.

This has the advantage of allowing every possible value of display
properties that's safe, while positively disabling any Lisp evaluation
while we process such properties that came from enriched text.  So we
are free from the need to figure out which forms of a display spec can
or cannot invoke Lisp.

WDYT?

Here's the patch:

--- src/xdisp.c~2	2017-09-07 11:16:30.503455400 +0300
+++ src/xdisp.c	2017-09-11 17:29:00.507991400 +0300
@@ -876,9 +876,9 @@ static int face_before_or_after_it_pos (
 static ptrdiff_t next_overlay_change (ptrdiff_t);
 static int handle_display_spec (struct it *, Lisp_Object, Lisp_Object,
 				Lisp_Object, struct text_pos *, ptrdiff_t, bool);
-static int handle_single_display_spec (struct it *, Lisp_Object,
-                                       Lisp_Object, Lisp_Object,
-                                       struct text_pos *, ptrdiff_t, int, bool);
+static int handle_single_display_spec (struct it *, Lisp_Object, Lisp_Object,
+				       Lisp_Object, struct text_pos *,
+				       ptrdiff_t, int, bool, bool);
 static int underlying_face_id (struct it *);
 
 #define face_before_it_pos(IT) face_before_or_after_it_pos (IT, true)
@@ -4731,6 +4731,14 @@ handle_display_spec (struct it *it, Lisp
 		     ptrdiff_t bufpos, bool frame_window_p)
 {
   int replacing = 0;
+  bool enable_eval = true;
+
+  /* Support (disable-eval PROP) which is used by enriched.el.  */
+  if (CONSP (spec) && EQ (XCAR (spec), Qdisable_eval))
+    {
+      enable_eval = false;
+      spec = XCAR (XCDR (spec));
+    }
 
   if (CONSP (spec)
       /* Simple specifications.  */
@@ -4754,7 +4762,8 @@ handle_display_spec (struct it *it, Lisp
 	{
 	  int rv = handle_single_display_spec (it, XCAR (spec), object,
 					       overlay, position, bufpos,
-					       replacing, frame_window_p);
+					       replacing, frame_window_p,
+					       enable_eval);
 	  if (rv != 0)
 	    {
 	      replacing = rv;
@@ -4772,7 +4781,8 @@ handle_display_spec (struct it *it, Lisp
 	{
 	  int rv = handle_single_display_spec (it, AREF (spec, i), object,
 					       overlay, position, bufpos,
-					       replacing, frame_window_p);
+					       replacing, frame_window_p,
+					       enable_eval);
 	  if (rv != 0)
 	    {
 	      replacing = rv;
@@ -4785,7 +4795,8 @@ handle_display_spec (struct it *it, Lisp
     }
   else
     replacing = handle_single_display_spec (it, spec, object, overlay, position,
-					    bufpos, 0, frame_window_p);
+					    bufpos, 0, frame_window_p,
+					    enable_eval);
   return replacing;
 }
 
@@ -4830,6 +4841,8 @@ display_prop_end (struct it *it, Lisp_Ob
    don't set up IT.  In that case, FRAME_WINDOW_P means SPEC
    is intended to be displayed in a window on a GUI frame.
 
+   Enable evaluation of Lisp forms only if ENABLE_EVAL_P is true.
+
    Value is non-zero if something was found which replaces the display
    of buffer or string text.  */
 
@@ -4837,7 +4850,7 @@ static int
 handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object,
 			    Lisp_Object overlay, struct text_pos *position,
 			    ptrdiff_t bufpos, int display_replaced,
-			    bool frame_window_p)
+			    bool frame_window_p, bool enable_eval_p)
 {
   Lisp_Object form;
   Lisp_Object location, value;
@@ -4855,6 +4868,8 @@ handle_single_display_spec (struct it *i
       spec = XCDR (spec);
     }
 
+  if (!NILP (form) && !EQ (form, Qt) && !enable_eval_p)
+    form = Qnil;
   if (!NILP (form) && !EQ (form, Qt))
     {
       ptrdiff_t count = SPECPDL_INDEX ();
@@ -4903,7 +4918,7 @@ handle_single_display_spec (struct it *i
 		    steps = - steps;
 		  it->face_id = smaller_face (it->f, it->face_id, steps);
 		}
-	      else if (FUNCTIONP (it->font_height))
+	      else if (FUNCTIONP (it->font_height) && enable_eval_p)
 		{
 		  /* Call function with current height as argument.
 		     Value is the new height.  */
@@ -4924,7 +4939,7 @@ handle_single_display_spec (struct it *i
 		  new_height = (XFLOATINT (it->font_height)
 				* XINT (f->lface[LFACE_HEIGHT_INDEX]));
 		}
-	      else
+	      else if (enable_eval_p)
 		{
 		  /* Evaluate IT->font_height with `height' bound to the
 		     current specified height to get the new height.  */
@@ -32164,6 +32179,10 @@ They are still logged to the *Messages* 
   DEFSYM (Qfontified, "fontified");
   DEFSYM (Qfontification_functions, "fontification-functions");
 
+  /* Name of the symbol which disables Lisp evaluation in 'display'
+     properties.  This is used by enriched.el.  */
+  DEFSYM (Qdisable_eval, "disable-eval");
+
   /* Name of the face used to highlight trailing whitespace.  */
   DEFSYM (Qtrailing_whitespace, "trailing-whitespace");
 




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 15:34:02 GMT) Full text and rfc822 format available.

Message #60 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: "Charles A. Roelli" <charles <at> aurox.ch>, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 11:33:36 -0400
I submitted this to https://github.com/distributedweaknessfiling/ .
I see you sent it to http://seclists.org/oss-sec/2017/q3/422 .
Are you sure this issue affects Emacs 19.29, as stated there?
The x-display code is "only" present since 21.1, AFAICS.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 16:33:02 GMT) Full text and rfc822 format available.

Message #63 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "Charles A. Roelli" <charles <at> aurox.ch>, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 12:32:38 -0400
Eli Zaretskii wrote:

>> At this point it seems that unsafe display specs are more the
>> exception than the rule, so it might make sense to define the
>> `enriched-display-prop-safe-p' function by excluding the unsafe
>> specifications instead of including the safe ones.  What do you
>> think?
>
> I'm not sure.  The display spec can be complex, so to make sure none
> of these exceptions sneak through, you will have to recursively unpack
> the spec data structure and examine each of the elements, which smells
> too similar to emulating 'eval'.  No?

FWIW, there is 'unsafep'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 16:39:01 GMT) Full text and rfc822 format available.

Message #66 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Glenn Morris <rgm <at> gnu.org>
Cc: "Charles A. Roelli" <charles <at> aurox.ch>, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 09:38:14 -0700
On 09/11/2017 08:33 AM, Glenn Morris wrote:
> I submitted this tohttps://github.com/distributedweaknessfiling/  .
> I see you sent it tohttp://seclists.org/oss-sec/2017/q3/422  .

Yes, I sent it to the oss-security mailing list, and it is archived here:

http://www.openwall.com/lists/oss-security/2017/09/11/1

> Are you sure this issue affects Emacs 19.29, as stated there?
> The x-display code is "only" present since 21.1, AFAICS.

Thanks for checking. When I wrote that, I looked for any of the text 
involved in Lars's patch. If a smaller patch will do, that might explain 
why you're seeing 21.1 rather than 19.29. We can mention 21.1 instead of 
19.29 in the 25.3 release, and I'll update etc/NEWS accordingly in 
emacs-25 and master once that comes out.

These days almost nobody is running Emacs older than 21.1, so the exact 
version number shouldn't matter to anybody other than software 
archaeologists.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 17:03:02 GMT) Full text and rfc822 format available.

Message #69 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Glenn Morris <rgm <at> gnu.org>
Cc: charles <at> aurox.ch, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 20:01:43 +0300
> From: Glenn Morris <rgm <at> gnu.org>
> Cc: charles <at> aurox.ch (Charles A. Roelli),  28350 <at> debbugs.gnu.org
> Date: Mon, 11 Sep 2017 12:32:38 -0400
> 
> FWIW, there is 'unsafep'.

Thanks.  Did that pass any audits from security experts?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 18:45:01 GMT) Full text and rfc822 format available.

Message #72 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, eggert <at> cs.ucla.edu, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 20:44:19 +0200
> Date: Mon, 11 Sep 2017 18:18:01 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> Anyway, I think I have a better idea for how to fix this on master.
> And I'm very sorry that this idea didn't come to me earlier, before
> you invested all these efforts in your patch.  (I'm also surprised
> that no one here had beaten me up to this idea.)
> 
> Here's the idea: we introduce a new form of a display property:
> 
>   ('disable-eval SPEC)
> 
> where SPEC is anything supported in a display property.

Thanks for suggesting this; it's much cleaner than sanitizing the
display specification from Lisp.  Looks good to me.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 19:09:02 GMT) Full text and rfc822 format available.

Message #75 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: larsi <at> gnus.org, eggert <at> cs.ucla.edu, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 22:07:26 +0300
> Date: Mon, 11 Sep 2017 20:44:19 +0200
> From: charles <at> aurox.ch (Charles A. Roelli)
> CC: eggert <at> cs.ucla.edu, larsi <at> gnus.org, 28350 <at> debbugs.gnu.org
> 
> > Here's the idea: we introduce a new form of a display property:
> > 
> >   ('disable-eval SPEC)
> > 
> > where SPEC is anything supported in a display property.
> 
> Thanks for suggesting this; it's much cleaner than sanitizing the
> display specification from Lisp.  Looks good to me.

Thanks, I will wait for a few days before pushing.

Thanks again for all your work on this grave issue.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Mon, 11 Sep 2017 21:17:02 GMT) Full text and rfc822 format available.

Message #78 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: "Charles A. Roelli" <charles <at> aurox.ch>, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Mon, 11 Sep 2017 17:16:11 -0400
Paul Eggert wrote:

> We can mention 21.1 instead of 19.29 in the 25.3 release, and I'll
> update etc/NEWS accordingly in emacs-25 and master once that comes
> out.

Too late. :(


For the record:

commit d9e28c1
Date:   Wed Jul 21 21:43:03 1999 +0000

    (enriched-translations): Add `display' and "x-display".

git describe --contains --tags d9e28c1
emacs-pretest-21.0.90~7452

> These days almost nobody is running Emacs older than 21.1, so the exact 
> version number shouldn't matter to anybody other than software 
> archaeologists.

Indeed not, but 19.29 v 21.1 is "staggeringly bad" v "extremely bad" for me.





bug Marked as fixed in versions 25.3. Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Mon, 11 Sep 2017 21:17:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Tue, 12 Sep 2017 20:00:02 GMT) Full text and rfc822 format available.

Message #83 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Glenn Morris <rgm <at> gnu.org>
Cc: "Charles A. Roelli" <charles <at> aurox.ch>, 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Tue, 12 Sep 2017 12:59:13 -0700
On 09/11/2017 02:16 PM, Glenn Morris wrote:
> Too late. :(

Yes, that horse left the barn. To close the barn door, I changed 
emacs-25's etc/NEWS (and master's etc/NEWS.25) to say "21.1" rather than 
"19.29" for when the bug was introduced, so that we look just "extremely 
bad" rather than "staggeringly bad" in the latest source code.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Thu, 14 Sep 2017 17:32:02 GMT) Full text and rfc822 format available.

Message #86 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Salvatore Bonaccorso <carnil <at> debian.org>
To: oss-security <at> lists.openwall.com
Subject: Re: [oss-security] GNU Emacs 25.2 enriched text remote code execution
Date: Thu, 14 Sep 2017 19:21:40 +0200
Hi

On Tue, Sep 12, 2017 at 07:22:51AM +0200, Salvatore Bonaccorso wrote:
> Hi
> 
> On Mon, Sep 11, 2017 at 08:58:57PM +0200, Salvatore Bonaccorso wrote:
> > Hi Paul,
> > 
> > On Sun, Sep 10, 2017 at 11:56:20PM -0700, Paul Eggert wrote:
> > > GNU Emacs is an extensible, customizable, free/libre text editor and
> > > software environment.  When Emacs renders MIME text/enriched data (Internet
> > > RFC 1896), it is vulnerable to arbitrary code execution. Since Emacs-based
> > > mail clients decode "Content-Type: text/enriched", this code is exploitable
> > > remotely. This bug affects GNU Emacs versions 19.29 through 25.2.
> > > 
> > > Although we know no efforts to exploit this in the wild, exploitation is easy.
> > [...]
> > > == Timeline ==
> > > 
> > > 2017-09-04. Bug reported to the Emacs bug tracker by Charles A. Roelli.
> > > 
> > > 2017-09-07. POC for remote code execution sent to the maintainers of Emacs
> > > and Gnus (Reiner Steib <Reiner.Steib <at> gmx.de>, private mail).
> > > 
> > > 2017-09-08. Patch (by Lars Ingebrigtsen <larsi <at> gnus.org>) to disable the
> > > problematic code and mitigation (private mail).
> > > 
> > > 2017-09-09. Patch committed in main development repository.
> > 
> > Have you requested a CVE for this issue?
> 
> FTR, it seems this was submitted to DWF already as per:
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28350#63

CVE-2017-14482 was assigned for this issue.

Regards,
Salvatore




Changed bug title to 'CVE-2017-14482: enriched.el code execution' from 'enriched.el code execution' Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Thu, 14 Sep 2017 17:35:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Thu, 14 Sep 2017 17:44:01 GMT) Full text and rfc822 format available.

Message #91 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Salvatore Bonaccorso <carnil <at> debian.org>
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: [oss-security] GNU Emacs 25.2 enriched text remote
 code execution
Date: Thu, 14 Sep 2017 13:43:02 -0400
Salvatore Bonaccorso wrote:

>> FTR, it seems this was submitted to DWF already as per:
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28350#63
>
> CVE-2017-14482 was assigned for this issue.

Thanks. Do I need to cancel or update the DWF submission (if so, how)?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#28350; Package emacs. (Thu, 14 Sep 2017 19:55:01 GMT) Full text and rfc822 format available.

Message #94 received at 28350 <at> debbugs.gnu.org (full text, mbox):

From: Salvatore Bonaccorso <carnil <at> debian.org>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 28350 <at> debbugs.gnu.org
Subject: Re: bug#28350: [oss-security] GNU Emacs 25.2 enriched text remote
 code execution
Date: Thu, 14 Sep 2017 21:54:21 +0200
Hi Glenn,

On Thu, Sep 14, 2017 at 01:43:02PM -0400, Glenn Morris wrote:
> Salvatore Bonaccorso wrote:
> 
> >> FTR, it seems this was submitted to DWF already as per:
> >> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28350#63
> >
> > CVE-2017-14482 was assigned for this issue.
> 
> Thanks. Do I need to cancel or update the DWF submission (if so, how)?

There is nothing further needed. The DWF has cancelled the request.




Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Sat, 16 Sep 2017 09:50:02 GMT) Full text and rfc822 format available.

Notification sent to charles <at> aurox.ch (Charles A. Roelli):
bug acknowledged by developer. (Sat, 16 Sep 2017 09:50:02 GMT) Full text and rfc822 format available.

Message #99 received at 28350-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: charles <at> aurox.ch, larsi <at> gnus.org
Cc: eggert <at> cs.ucla.edu, 28350-done <at> debbugs.gnu.org
Subject: Re: bug#28350: enriched.el code execution
Date: Sat, 16 Sep 2017 12:48:58 +0300
> Date: Mon, 11 Sep 2017 22:07:26 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: larsi <at> gnus.org, eggert <at> cs.ucla.edu, 28350 <at> debbugs.gnu.org
> 
> > Date: Mon, 11 Sep 2017 20:44:19 +0200
> > From: charles <at> aurox.ch (Charles A. Roelli)
> > CC: eggert <at> cs.ucla.edu, larsi <at> gnus.org, 28350 <at> debbugs.gnu.org
> > 
> > > Here's the idea: we introduce a new form of a display property:
> > > 
> > >   ('disable-eval SPEC)
> > > 
> > > where SPEC is anything supported in a display property.
> > 
> > Thanks for suggesting this; it's much cleaner than sanitizing the
> > display specification from Lisp.  Looks good to me.
> 
> Thanks, I will wait for a few days before pushing.

Done.

Lars, I re-enabled support for enriched text in Gnus, as the
vulnerability is now removed.  Feel free to disable it again, if you
don't want Gnus users to be able to display enriched text, ever.

I'm marking the bug done.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 14 Oct 2017 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 189 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.