unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Security flaw in enable-local-eval; new release plan
@ 2012-08-13  3:10 Chong Yidong
  2012-08-13  6:32 ` Thien-Thi Nguyen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chong Yidong @ 2012-08-13  3:10 UTC (permalink / raw)
  To: emacs-devel

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

Paul Ling has found a security flaw in the file-local variables code in
GNU Emacs.  When the user option `enable-local-variables' is set to
`:safe' (the default value is t), Emacs should automatically refuse to
evaluate `eval' forms in file-local variable sections.  Due to the bug,
Emacs instead automatically evaluates such `eval' forms.

Thus, if the user changes the value of `enable-local-variables' to
`:safe', visiting a malicious file can cause automatic execution of
arbitrary Emacs Lisp code with the permissions of the user.

The bug is present in Emacs 23.2, 23.3, 23.4, and 24.1.

Attached are patches to fix this bug for Emacs 23.4 and Emacs 24.1,
written by Glenn Morris.  (The 23.4 patch should apply to the rest of
the Emacs 23.x series.)

Due to this problem, I plan to make a 24.2 release from the emacs-24
development branch.  The fix has already been committed to the emacs-24
branch, as well as the trunk.  I would like to make the release in a few
days; say, by the end of the week.  If there are any obviously-safe
bugfixes in the trunk which you would like to see backported to the
emacs-24 branch, please point these out ASAP.  Note also that this means
the current development trunk will be scheduled for Emacs 24.3.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: local-vars-patch-23.4.patch --]
[-- Type: text/x-diff, Size: 1373 bytes --]

*** emacs-23.4/lisp/files.el~	2012-08-13 10:50:42.340171173 +0800
--- emacs-23.4/lisp/files.el	2012-08-13 10:51:12.528172450 +0800
***************
*** 2986,2996 ****
  	      ;; Obey `enable-local-eval'.
  	      ((eq var 'eval)
  	       (when enable-local-eval
! 		 (push elt all-vars)
! 		 (or (eq enable-local-eval t)
! 		     (hack-one-local-variable-eval-safep (eval (quote val)))
! 		     (safe-local-variable-p var val)
! 		     (push elt unsafe-vars))))
  	      ;; Ignore duplicates (except `mode') in the present list.
  	      ((and (assq var all-vars) (not (eq var 'mode))) nil)
  	      ;; Accept known-safe variables.
--- 2986,3001 ----
  	      ;; Obey `enable-local-eval'.
  	      ((eq var 'eval)
  	       (when enable-local-eval
! 		 (let ((safe (or (hack-one-local-variable-eval-safep
! 				  (eval (quote val)))
! 				 ;; In case previously marked safe (bug#5636).
! 				 (safe-local-variable-p var val))))
! 		   ;; If not safe and e-l-v = :safe, ignore totally.
! 		   (when (or safe (not (eq enable-local-variables :safe)))
! 		     (push elt all-vars)
! 		     (or (eq enable-local-eval t)
! 			 safe
! 			 (push elt unsafe-vars))))))
  	      ;; Ignore duplicates (except `mode') in the present list.
  	      ((and (assq var all-vars) (not (eq var 'mode))) nil)
  	      ;; Accept known-safe variables.

Diff finished.  Mon Aug 13 10:53:45 2012

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: local-vars-patch-24.1.patch --]
[-- Type: text/x-diff, Size: 1373 bytes --]

*** emacs-24.1/lisp/files.el~	2012-08-13 10:54:50.256181658 +0800
--- emacs-24.1/lisp/files.el	2012-08-13 10:54:58.076181990 +0800
***************
*** 3107,3117 ****
  	      ;; Obey `enable-local-eval'.
  	      ((eq var 'eval)
  	       (when enable-local-eval
! 		 (push elt all-vars)
! 		 (or (eq enable-local-eval t)
! 		     (hack-one-local-variable-eval-safep (eval (quote val)))
! 		     (safe-local-variable-p var val)
! 		     (push elt unsafe-vars))))
  	      ;; Ignore duplicates (except `mode') in the present list.
  	      ((and (assq var all-vars) (not (eq var 'mode))) nil)
  	      ;; Accept known-safe variables.
--- 3107,3122 ----
  	      ;; Obey `enable-local-eval'.
  	      ((eq var 'eval)
  	       (when enable-local-eval
! 		 (let ((safe (or (hack-one-local-variable-eval-safep
! 				  (eval (quote val)))
! 				 ;; In case previously marked safe (bug#5636).
! 				 (safe-local-variable-p var val))))
! 		   ;; If not safe and e-l-v = :safe, ignore totally.
! 		   (when (or safe (not (eq enable-local-variables :safe)))
! 		     (push elt all-vars)
! 		     (or (eq enable-local-eval t)
! 			 safe
! 			 (push elt unsafe-vars))))))
  	      ;; Ignore duplicates (except `mode') in the present list.
  	      ((and (assq var all-vars) (not (eq var 'mode))) nil)
  	      ;; Accept known-safe variables.

Diff finished.  Mon Aug 13 10:55:03 2012

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Security flaw in enable-local-eval; new release plan
  2012-08-13  3:10 Security flaw in enable-local-eval; new release plan Chong Yidong
@ 2012-08-13  6:32 ` Thien-Thi Nguyen
  2012-08-13 12:45   ` Herring, Davis
  2012-08-13  6:49 ` Glenn Morris
  2012-08-13  8:06 ` Ulrich Mueller
  2 siblings, 1 reply; 6+ messages in thread
From: Thien-Thi Nguyen @ 2012-08-13  6:32 UTC (permalink / raw)
  To: Chong Yidong; +Cc: emacs-devel

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

() Chong Yidong <cyd@gnu.org>
() Mon, 13 Aug 2012 11:10:57 +0800

   (let ((safe (or (hack-one-local-variable-eval-safep
                    (eval (quote val)))
                   ;; In case previously marked safe (bug#5636).
                   (safe-local-variable-p var val))))
     ;; If not safe and e-l-v = :safe, ignore totally.
     (when (or safe (not (eq enable-local-variables :safe)))
       (push elt all-vars)
       (or (eq enable-local-eval t)
           safe
           (push elt unsafe-vars))))

It seems control reaches ‘eval’ before reaching the ‘:safe’ check, thus
defeating the check.  Am i missing something?

-- 
Thien-Thi Nguyen ..................................... GPG key: 4C807502
.                  NB: ttn at glug dot org is not me                   .
.                 (and has not been since 2007 or so)                  .
.                        ACCEPT NO SUBSTITUTES                         .
........... please send technical questions to mailing lists ...........

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Security flaw in enable-local-eval; new release plan
  2012-08-13  3:10 Security flaw in enable-local-eval; new release plan Chong Yidong
  2012-08-13  6:32 ` Thien-Thi Nguyen
@ 2012-08-13  6:49 ` Glenn Morris
  2012-08-13  8:06 ` Ulrich Mueller
  2 siblings, 0 replies; 6+ messages in thread
From: Glenn Morris @ 2012-08-13  6:49 UTC (permalink / raw)
  To: Chong Yidong; +Cc: emacs-devel

Chong Yidong wrote:

> If there are any obviously-safe bugfixes in the trunk which you would
> like to see backported to the emacs-24 branch, please point these out
> ASAP.

I think it would be nice if someone could review
http://debbugs.gnu.org/11712



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Security flaw in enable-local-eval; new release plan
  2012-08-13  3:10 Security flaw in enable-local-eval; new release plan Chong Yidong
  2012-08-13  6:32 ` Thien-Thi Nguyen
  2012-08-13  6:49 ` Glenn Morris
@ 2012-08-13  8:06 ` Ulrich Mueller
  2012-08-14  4:07   ` Chong Yidong
  2 siblings, 1 reply; 6+ messages in thread
From: Ulrich Mueller @ 2012-08-13  8:06 UTC (permalink / raw)
  To: Chong Yidong; +Cc: emacs-devel

>>>>> On Mon, 13 Aug 2012, Chong Yidong wrote:

> Paul Ling has found a security flaw in the file-local variables code
> in GNU Emacs.

Is there already a CVE number for this?

Ulrich



^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: Security flaw in enable-local-eval; new release plan
  2012-08-13  6:32 ` Thien-Thi Nguyen
@ 2012-08-13 12:45   ` Herring, Davis
  0 siblings, 0 replies; 6+ messages in thread
From: Herring, Davis @ 2012-08-13 12:45 UTC (permalink / raw)
  To: Thien-Thi Nguyen, Chong Yidong; +Cc: emacs-devel@gnu.org

>    (let ((safe (or (hack-one-local-variable-eval-safep
>                     (eval (quote val)))
>                    ;; In case previously marked safe (bug#5636).
>                    (safe-local-variable-p var val))))
> [...]
> It seems control reaches ‘eval’ before reaching the ‘:safe’ check, thus
> defeating the check.  Am i missing something?

The `eval' is of a `quote', so I don't know why it's not just `val'.  Meanwhile, calling `safe-local-variable-p' for the `eval' pseudo-variable seems wrong to me, even though by default nothing is safe for it (and it would be insane to put an entry, nominally for `eval''s value as a variable, in `safe-local-variable-values').

Davis


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Security flaw in enable-local-eval; new release plan
  2012-08-13  8:06 ` Ulrich Mueller
@ 2012-08-14  4:07   ` Chong Yidong
  0 siblings, 0 replies; 6+ messages in thread
From: Chong Yidong @ 2012-08-14  4:07 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: emacs-devel

Ulrich Mueller <ulm@gentoo.org> writes:

>>>>>> On Mon, 13 Aug 2012, Chong Yidong wrote:
>
>> Paul Ling has found a security flaw in the file-local variables code
>> in GNU Emacs.
>
> Is there already a CVE number for this?

Use CVE-2012-3479 for this issue.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-08-14  4:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-13  3:10 Security flaw in enable-local-eval; new release plan Chong Yidong
2012-08-13  6:32 ` Thien-Thi Nguyen
2012-08-13 12:45   ` Herring, Davis
2012-08-13  6:49 ` Glenn Morris
2012-08-13  8:06 ` Ulrich Mueller
2012-08-14  4:07   ` Chong Yidong

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).