[mwelinder@gmail.com: Emacs security bug]

[mwelinder@gmail.com: Emacs security bug]

[Eli Zaretskii]

------- Start of forwarded message -------
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00 autolearn=ham 
	version=3.1.0
Date: Fri, 9 May 2008 12:45:25 -0400
From: "Morten Welinder" <mwelinder@gmail.com>
To: eliz@gnu.org
Subject: Emacs security bug

Hi there,

it's been a while or two -- DJGPP was hot, new technology when we last
spoke, :-)

It's unclear to me where to send Emacs security concerns, so I am sending
this one to you.  Please forward appropriately.

1. Create .emacs with contents
    (global-font-lock-mode t)
    (seq font-lock-support-mode 'fast-lock-mode)

2. Create foo.c with contents /* Nothing to see here */

3. Create foo.c.flc with contents (message "Something to see here!")

4. Start Emacs and load foo.c

- --> Observe that code from foo.c.flc is run.  Not good.
(This is with Emacs 21.3.1; XEmacs is also affected, although step 1 needs to
be adjusted.)

Suggestions:

a. Remove "." from fast-lock-cache-directories.  Littering little
files everywhere
    is not a good idea anyway.

b. Don't use load to handle the .flc file.  Instead read it into a
buffer and read
    one s-expression at a time and verify that it is sane before evaluating it.

c. Don't use files owned by anyone else.  This cannot stand alone, though, as
    it has a race condition.

Morten Welinder
------- End of forwarded message -------

Re: [mwelinder@gmail.com: Emacs security bug]

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> From: "Morten Welinder" <mwelinder@gmail.com>
>
> 1. Create .emacs with contents
>     (global-font-lock-mode t)
>     (seq font-lock-support-mode 'fast-lock-mode)
>
> 2. Create foo.c with contents /* Nothing to see here */
>
> 3. Create foo.c.flc with contents (message "Something to see here!")
>
> 4. Start Emacs and load foo.c
>
> - --> Observe that code from foo.c.flc is run.  Not good.  (This is
>with Emacs 21.3.1; XEmacs is also affected, although step 1 needs to be
>adjusted.)

fast-lock-mode has been obsolete since Emacs 22...

Re: [mwelinder@gmail.com: Emacs security bug]

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Fri, 09 May 2008 19:19:20 -0400
> Cc: emacs-devel@gnu.org
> 
> fast-lock-mode has been obsolete since Emacs 22...

But people could still use it.

In any case, please include Morten in the CC list of this thread.

Re: [mwelinder@gmail.com: Emacs security bug]

[Christian Faulhammer]

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

Hallo,

Eli Zaretskii <eliz@gnu.org>:

> > From: Chong Yidong <cyd@stupidchicken.com>
> > Date: Fri, 09 May 2008 19:19:20 -0400
> > Cc: emacs-devel@gnu.org
> > 
> > fast-lock-mode has been obsolete since Emacs 22...
> 
> But people could still use it.
> 
> In any case, please include Morten in the CC list of this thread.

 And some distributions ship it and support it.

V-Li

-- 
Christian Faulhammer, Gentoo Lisp project
<URL:http://www.gentoo.org/proj/en/lisp/>, #gentoo-lisp on FreeNode

<URL:http://www.faulhammer.org/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [mwelinder@gmail.com: Emacs security bug]

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> From: "Morten Welinder" <mwelinder@gmail.com>
>
> 1. Create .emacs with contents
>     (global-font-lock-mode t)
>     (seq font-lock-support-mode 'fast-lock-mode)
>
> 2. Create foo.c with contents /* Nothing to see here */
>
> 3. Create foo.c.flc with contents (message "Something to see here!")
>
> 4. Start Emacs and load foo.c
>
> - --> Observe that code from foo.c.flc is run.  Not good.
> (This is with Emacs 21.3.1; XEmacs is also affected, although step 1 needs to
> be adjusted.)
>
> Suggestions:
>
> a. Remove "." from fast-lock-cache-directories.  Littering little
> files everywhere is not a good idea anyway.
>
> b. Don't use load to handle the .flc file.  Instead read it into a
> buffer and read one s-expression at a time and verify that it is sane
> before evaluating it.

Simon, could you take a look at this (you're listed as the author of
fast-lock.el)?

Re: [mwelinder@gmail.com: Emacs security bug]

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> Date: Sat, 10 May 2008 10:50:17 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > From: "Morten Welinder" <mwelinder@gmail.com>
> >
> > 1. Create .emacs with contents
> >     (global-font-lock-mode t)
> >     (seq font-lock-support-mode 'fast-lock-mode)
> >
> > 2. Create foo.c with contents /* Nothing to see here */
> >
> > 3. Create foo.c.flc with contents (message "Something to see here!")
> >
> > 4. Start Emacs and load foo.c
> >
> > - --> Observe that code from foo.c.flc is run.  Not good.
> > (This is with Emacs 21.3.1; XEmacs is also affected, although step 1 needs to
> > be adjusted.)
> >
> > Suggestions:
> >
> > a. Remove "." from fast-lock-cache-directories.  Littering little
> > files everywhere is not a good idea anyway.
> >
> > b. Don't use load to handle the .flc file.  Instead read it into a
> > buffer and read one s-expression at a time and verify that it is sane
> > before evaluating it.
> 
> Simon, could you take a look at this (you're listed as the author of
> fast-lock.el)?

Please keep Morten on the CC list of this thread.  I don't want to
have to forward messages back and forth forever.

RE: [mwelinder@gmail.com: Emacs security bug]

[Marshall, Simon]

> > - --> Observe that code from foo.c.flc is run.  Not good.
> > (This is with Emacs 21.3.1; XEmacs is also affected, although step 1
needs to
> > be adjusted.)
> >
> > Suggestions:
> >
> > a. Remove "." from fast-lock-cache-directories.  Littering little
> > files everywhere is not a good idea anyway.
> >
> > b. Don't use load to handle the .flc file.  Instead read it into a
> > buffer and read one s-expression at a time and verify that it is
sane
> > before evaluating it.
> 
> Simon, could you take a look at this (you're listed as the author of
> fast-lock.el)?

OK, it seems fast-lock writes a single (fast-lock-cache-data ...) form
into the .flc file, and fast-lock loads the .flc file.  Is there a way
to restrict the forms evaluated during a load, rather than parsing the
.flc file?  Or a better way?

Also note that fast-lock-cache-data calls font-lock-compile-keywords
(when setting font-lock-keywords amongst others) which can ultimately
eval a keyword.  That would also be a security hole, right?  (I assume
that isn't a security issue in general because font-lock-keywords is a
risky local variable, but I think that is a security issue here.)
Someone could put their nasty form in the top-level call of
fast-lock-cache-data, for font-lock-compile-keyword to eval, rather than
at the top-level of the .flc file itself.  I don't see how that could be
fixed with the current design of font-lock.el.

I know this is a bit cheeky of me, but fast-lock.el is so old that even
its successor, lazy-lock.el, is in lisp/obsolete.  It was last updated
for functional purposes in 1996.  

Probably the most reasonable fix, in the circumstances, is to make
fast-lock-cache-directories a risky local variable and remove "." from
its default value?

Simon.


 "Misys" is the trade name for Misys plc (registered in England and Wales). Registration Number: 01360027. Registered office: Burleigh House, Chapel Oak, Salford Priors, Evesham WR11 8SP. For a list of Misys group operating companies please go to http://www.misys.com/html/about_us/group_operating_companies/. This email and any attachments have been scanned for known viruses using multiple scanners. 
 
We believe that this email and any attachments are virus free, however the recipient must take full responsibility for virus checking. This email message is intended for the named recipient only. It may be privileged and/or confidential. If you are not the named recipient of this email please notify us immediately and do not copy it or use it for any purpose, nor disclose its contents to any other person. This email does not constitute the commencement of legal relations between you and Misys plc. Please refer to the executed contract between you and the relevant member of the Misys group for the identity of the contracting party with which you are dealing. 

Re: [mwelinder@gmail.com: Emacs security bug]

[Stefan Monnier]

> OK, it seems fast-lock writes a single (fast-lock-cache-data ...) form
> into the .flc file, and fast-lock loads the .flc file.  Is there a way
> to restrict the forms evaluated during a load, rather than parsing the
> .flc file?  Or a better way?

I see no point in investing any significant time to try and fix
this problem.

> Probably the most reasonable fix, in the circumstances, is to make
> fast-lock-cache-directories a risky local variable and remove "." from
> its default value?

Sounds OK.


        Stefan

RE: [mwelinder@gmail.com: Emacs security bug]

[Marshall, Simon]

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

> I see no point in investing any significant time to try and fix
> this problem.

Phew.

> > Probably the most reasonable fix, in the circumstances, is to make
> > fast-lock-cache-directories a risky local variable and remove "."
from
> > its default value?
> 
> Sounds OK.

Attached is a patch.  Simon.



 "Misys" is the trade name for Misys plc (registered in England and Wales). Registration Number: 01360027. Registered office: Burleigh House, Chapel Oak, Salford Priors, Evesham WR11 8SP. For a list of Misys group operating companies please go to http://www.misys.com/html/about_us/group_operating_companies/. This email and any attachments have been scanned for known viruses using multiple scanners. 
 
We believe that this email and any attachments are virus free, however the recipient must take full responsibility for virus checking. This email message is intended for the named recipient only. It may be privileged and/or confidential. If you are not the named recipient of this email please notify us immediately and do not copy it or use it for any purpose, nor disclose its contents to any other person. This email does not constitute the commencement of legal relations between you and Misys plc. Please refer to the executed contract between you and the relevant member of the Misys group for the identity of the contracting party with which you are dealing. 

[-- Attachment #2: fast-lock.diff --]
[-- Type: application/octet-stream, Size: 2124 bytes --]

*** fast-lock.el.~1~	Thu Jan 10 12:15:40 2008
--- fast-lock.el	Mon May 12 17:30:28 2008
***************
*** 286,292 ****
  				      (integer :tag "size")))))
    :group 'fast-lock)
  
! (defcustom fast-lock-cache-directories '("." "~/.emacs-flc")
  ; - `internal', keep each file's Font Lock cache file in the same file.
  ; - `external', keep each file's Font Lock cache file in the same directory.
    "*Directories in which Font Lock cache files are saved and read.
--- 286,292 ----
  				      (integer :tag "size")))))
    :group 'fast-lock)
  
! (defcustom fast-lock-cache-directories '("~/.emacs-flc")
  ; - `internal', keep each file's Font Lock cache file in the same file.
  ; - `external', keep each file's Font Lock cache file in the same directory.
    "*Directories in which Font Lock cache files are saved and read.
***************
*** 304,315 ****
   ((\"^/your/true/home/directory/\" . \".\") \"~/.emacs-flc\")
  
  would cause a file's current directory to be used if the file is under your
! home directory hierarchy, or otherwise the absolute directory `~/.emacs-flc'."
    :type '(repeat (radio (directory :tag "directory")
  			(cons :tag "Matching"
  			      (regexp :tag "regexp")
  			      (directory :tag "directory"))))
    :group 'fast-lock)
  
  (defcustom fast-lock-save-events '(kill-buffer kill-emacs)
    "*Events under which caches will be saved.
--- 304,318 ----
   ((\"^/your/true/home/directory/\" . \".\") \"~/.emacs-flc\")
  
  would cause a file's current directory to be used if the file is under your
! home directory hierarchy, or otherwise the absolute directory `~/.emacs-flc'.
! For security reasons, it is not advisable to use the file's current directory
! to avoid the possibility of using the cache of another user."
    :type '(repeat (radio (directory :tag "directory")
  			(cons :tag "Matching"
  			      (regexp :tag "regexp")
  			      (directory :tag "directory"))))
    :group 'fast-lock)
+ (put 'fast-lock-cache-directories 'risky-local-variable t)
  
  (defcustom fast-lock-save-events '(kill-buffer kill-emacs)
    "*Events under which caches will be saved.

[-- Attachment #3: ChangeLog.diff --]
[-- Type: application/octet-stream, Size: 389 bytes --]

*** ChangeLog~	Wed Mar 26 13:32:44 2008
--- ChangeLog	Mon May 12 17:33:38 2008
***************
*** 1,3 ****
--- 1,8 ----
+ 2008-05-12  Simon Marshall  <simon@gnu.org>
+ 
+ 	* fast-lock.el (fast-lock-cache-directories): Remove "." from its
+ 	default value and give it the risky-local-variable property.
+ 
  2008-03-26  Chong Yidong  <cyd@stupidchicken.com>
  
  	* Version 22.2 released.

Re: [mwelinder@gmail.com: Emacs security bug]

[Stefan Monnier]

> Attached is a patch.  Simon.

Thanks, installed,


        Stefan

RE: [mwelinder@gmail.com: Emacs security bug]

[Ulrich Mueller]

>>>>> On Mon, 12 May 2008, Marshall, Simon wrote:

>> > Probably the most reasonable fix, in the circumstances, is to
>> > make fast-lock-cache-directories a risky local variable and
>> > remove "." from its default value?
>> 
>> Sounds OK.

> Attached is a patch.  Simon.

> + (put 'fast-lock-cache-directories 'risky-local-variable t)

I think this will not work, since above line is only executed after
fast-lock has been loaded. So the symbol may not yet have the property
when the file variables are evaluated.

It would work if above line was moved to files.el.

Ulrich

RE: [mwelinder@gmail.com: Emacs security bug]

[Ulrich Mueller]

>>>>> On Mon, 12 May 2008, Ulrich Mueller wrote:

>> + (put 'fast-lock-cache-directories 'risky-local-variable t)

> I think this will not work, since above line is only executed after
> fast-lock has been loaded. So the symbol may not yet have the property
> when the file variables are evaluated.

> It would work if above line was moved to files.el.

An autoload cookie would also be sufficient, of course.

Ulrich

RE: [mwelinder@gmail.com: Emacs security bug]

[Marshall, Simon]

> >> + (put 'fast-lock-cache-directories 'risky-local-variable t)
> 
> > I think this will not work, since above line is only executed after
> > fast-lock has been loaded. So the symbol may not yet have the
property
> > when the file variables are evaluated.
> 
> > It would work if above line was moved to files.el.
> 
> An autoload cookie would also be sufficient, of course.

I did wonder about this, but when I looked at how other files were
adding the property, I noticed many of them did not have an autoload
cookie...


 "Misys" is the trade name for Misys plc (registered in England and Wales). Registration Number: 01360027. Registered office: Burleigh House, Chapel Oak, Salford Priors, Evesham WR11 8SP. For a list of Misys group operating companies please go to http://www.misys.com/html/about_us/group_operating_companies/. This email and any attachments have been scanned for known viruses using multiple scanners. 
 
We believe that this email and any attachments are virus free, however the recipient must take full responsibility for virus checking. This email message is intended for the named recipient only. It may be privileged and/or confidential. If you are not the named recipient of this email please notify us immediately and do not copy it or use it for any purpose, nor disclose its contents to any other person. This email does not constitute the commencement of legal relations between you and Misys plc. Please refer to the executed contract between you and the relevant member of the Misys group for the identity of the contracting party with which you are dealing. 

RE: [mwelinder@gmail.com: Emacs security bug]

[Ulrich Mueller]

>>>>> On Tue, 13 May 2008, Marshall, Simon wrote:

>> > + (put 'fast-lock-cache-directories 'risky-local-variable t)

>> I think this will not work, since above line is only executed
>> after fast-lock has been loaded. So the symbol may not yet have
>> the property when the file variables are evaluated.

>> An autoload cookie would also be sufficient, of course.

> I did wonder about this, but when I looked at how other files were
> adding the property, I noticed many of them did not have an autoload
> cookie...

The autoload cookie would cause the "put" form to appear in the
autoloads of fast-lock. (In the context of Emacs 21, it would then be
included in loaddefs.el.)

However, as Stefan already noted, it's not really relevant for
Emacs 22 or later. And fast-lock isn't even included in loaddefs.el
anymore.

Ulrich

RE: [mwelinder@gmail.com: Emacs security bug]

[Marshall, Simon]

> >> An autoload cookie would also be sufficient, of course.
> 
> > I did wonder about this, but when I looked at how other files were
> > adding the property, I noticed many of them did not have an autoload
> > cookie...
> 
> The autoload cookie would cause the "put" form to appear in the
> autoloads of fast-lock. (In the context of Emacs 21, it would then be
> included in loaddefs.el.)

Well, yes, though my point was only that many others do not do that.  

So, if anyone is worried about the lack of an autoload cookie before the
put form in fast-lock.el, then there are many other places they can get
nervous about.

Simon.


 "Misys" is the trade name for Misys plc (registered in England and Wales). Registration Number: 01360027. Registered office: Burleigh House, Chapel Oak, Salford Priors, Evesham WR11 8SP. For a list of Misys group operating companies please go to http://www.misys.com/html/about_us/group_operating_companies/. This email and any attachments have been scanned for known viruses using multiple scanners. 
 
We believe that this email and any attachments are virus free, however the recipient must take full responsibility for virus checking. This email message is intended for the named recipient only. It may be privileged and/or confidential. If you are not the named recipient of this email please notify us immediately and do not copy it or use it for any purpose, nor disclose its contents to any other person. This email does not constitute the commencement of legal relations between you and Misys plc. Please refer to the executed contract between you and the relevant member of the Misys group for the identity of the contracting party with which you are dealing. 

Re: [mwelinder@gmail.com: Emacs security bug]

[Stefan Monnier]

>> + (put 'fast-lock-cache-directories 'risky-local-variable t)

> I think this will not work, since above line is only executed after
> fast-lock has been loaded. So the symbol may not yet have the property
> when the file variables are evaluated.

> It would work if above line was moved to files.el.

It's not that important anyway: since Emacs-22, all variables are
presumed unsafe unless explicitly marked as safe.


        Stefan

Re: [mwelinder@gmail.com: Emacs security bug]

[Florian Weimer]

* Simon Marshall:

> Probably the most reasonable fix, in the circumstances, is to make
> fast-lock-cache-directories a risky local variable and remove "." from
> its default value?

Is it actually necessary to have a cache?  Can't it just be removed?
Or is it an essential part of the code?

RE: [mwelinder@gmail.com: Emacs security bug]

[Marshall, Simon]

> > Probably the most reasonable fix, in the circumstances, is to make
> > fast-lock-cache-directories a risky local variable and remove "."
from
> > its default value?
> 
> Is it actually necessary to have a cache?  Can't it just be removed?
> Or is it an essential part of the code?

The purpose of fast-lock was to cache buffer fontification, in the days
before window fontification.  So, the cache was the whole point.

Simon.


 "Misys" is the trade name for Misys plc (registered in England and Wales). Registration Number: 01360027. Registered office: Burleigh House, Chapel Oak, Salford Priors, Evesham WR11 8SP. For a list of Misys group operating companies please go to http://www.misys.com/html/about_us/group_operating_companies/. This email and any attachments have been scanned for known viruses using multiple scanners. 
 
We believe that this email and any attachments are virus free, however the recipient must take full responsibility for virus checking. This email message is intended for the named recipient only. It may be privileged and/or confidential. If you are not the named recipient of this email please notify us immediately and do not copy it or use it for any purpose, nor disclose its contents to any other person. This email does not constitute the commencement of legal relations between you and Misys plc. Please refer to the executed contract between you and the relevant member of the Misys group for the identity of the contracting party with which you are dealing.