Security flaw in EDE; new release plans

Security flaw in EDE; new release plans

[Chong Yidong]

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

Hiroshi Oota has found a security flaw in EDE (part of CEDET), a
development tool included in Emacs.  EDE can store various information
about a project, such as how to build the project, in a file named
Project.ede in the project directory tree.  When the minor mode
`global-ede-mode' is enabled, visiting a file causes Emacs to look for
Project.ede in the file's directory or one of its parent directories.
If Project.ede is present, Emacs automatically reads and evaluates the
first Lisp expression in it.

This design exposes EDE users to the danger of loading malicious code
from one file (Project.ede), simply by visiting another file in the same
directory tree.

A patch to fix this problem, for the Emacs 23.3 release, is attached.
It prevents EDE from loading Project.ede files, except in directories
explicitly designated as "safe" by the user via the new list variable
`ede-project-directories'.  The value of this variable is initially the
empty list; Emacs offers to add to it when the user invokes the `M-x
ede' or `M-x ede-new' command.  EDE project types that do not use
Project.ede (e.g. those that scan makefiles for build information) are
unaffected, since they do not involve loading Lisp code.


Due to this problem, we will make a 23.4 release from the emacs-23
branch.  I have committed the above fix to the branch, and will shortly
commit another more complicated fix from Eric Ludlam which tries to
prevent CEDET classes from loading unsafe forms at all.  In a few days,
I will make the 23.3.90 pretest; during this brief window, if anyone
thinks there is another bug fix that ought to go into 23.4, please
promptly raise the issue on emacs-devel---but we will be very
conservative about allowing commits, in order to release 23.4 ASAP.  If
no serious problems are found with the 23.3.90 pretest, the 23.4 release
will probably follow right after.

I will also make a new Emacs 24 pretest (24.0.93) soon, incorporating
the same fixes.  Until then, Emacs pretesters should be aware of this
security flaw if they use EDE.  The upstream CEDET project will also be
making a new security release.

Thanks to Hiroshi Oota for pointing out the flaw, and to Eric Ludlam and
David Engster for working on the fix.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ede-project-fix.patch --]
[-- Type: text/x-diff, Size: 17600 bytes --]

diff -r -c emacs-23.3-old/lisp/cedet/ede/auto.el emacs-23.3/lisp/cedet/ede/auto.el
*** emacs-23.3-old/lisp/cedet/ede/auto.el	2011-01-09 01:45:14.000000000 +0800
--- emacs-23.3/lisp/cedet/ede/auto.el	2012-01-09 13:20:12.410502713 +0800
***************
*** 58,63 ****
--- 58,70 ----
  	  :initform t
  	  :documentation
  	  "Non-nil if this is an option when a user creates a project.")
+    (safe-p :initarg :safe-p
+ 	   :initform t
+ 	   :documentation
+ 	   "Non-nil if the project load files are \"safe\".
+ An unsafe project is one that loads project variables via Emacs
+ Lisp code.  A safe project is one that loads project variables by
+ scanning files without loading Lisp code from them.")
     )
    "Class representing minimal knowledge set to run preliminary EDE functions.
  When more advanced functionality is needed from a project type, that projects
***************
*** 69,81 ****
  			 :name "Make" :file 'ede/proj
  			 :proj-file "Project.ede"
  			 :load-type 'ede-proj-load
! 			 :class-sym 'ede-proj-project)
     (ede-project-autoload "edeproject-automake"
  			 :name "Automake" :file 'ede/proj
  			 :proj-file "Project.ede"
  			 :initializers '(:makefile-type Makefile.am)
  			 :load-type 'ede-proj-load
! 			 :class-sym 'ede-proj-project)
     (ede-project-autoload "automake"
  			 :name "automake" :file 'ede/project-am
  			 :proj-file "Makefile.am"
--- 76,90 ----
  			 :name "Make" :file 'ede/proj
  			 :proj-file "Project.ede"
  			 :load-type 'ede-proj-load
! 			 :class-sym 'ede-proj-project
! 			 :safe-p nil)
     (ede-project-autoload "edeproject-automake"
  			 :name "Automake" :file 'ede/proj
  			 :proj-file "Project.ede"
  			 :initializers '(:makefile-type Makefile.am)
  			 :load-type 'ede-proj-load
! 			 :class-sym 'ede-proj-project
! 			 :safe-p nil)
     (ede-project-autoload "automake"
  			 :name "automake" :file 'ede/project-am
  			 :proj-file "Makefile.am"
***************
*** 84,89 ****
--- 93,100 ----
  			 :new-p nil))
    "List of vectors defining how to determine what type of projects exist.")
  
+ (put 'ede-project-class-files 'risky-local-variable t)
+ 
  ;;; EDE project-autoload methods
  ;;
  (defmethod ede-project-root ((this ede-project-autoload))
***************
*** 122,127 ****
--- 133,151 ----
      (when (and f (file-exists-p f))
        f)))
  
+ (defmethod ede-auto-load-project ((this ede-project-autoload) dir)
+   "Load in the project associated with THIS project autoload description.
+ THIS project description should be valid for DIR, where the project will
+ be loaded."
+   ;; Last line of defense: don't load unsafe projects.
+   (when (not (or (oref this :safe-p)
+ 		 (ede-directory-safe-p dir)))
+     (error "Attempt to load an unsafe project (bug elsewhere in EDE)"))
+   ;; Things are good - so load the project.
+   (let ((o (funcall (oref this load-type) dir)))
+     (when (not o)
+       (error "Project type error: :load-type failed to create a project"))
+     (ede-add-project-to-global-list o)))
  
  (provide 'ede/auto)
  
diff -r -c emacs-23.3-old/lisp/cedet/ede/simple.el emacs-23.3/lisp/cedet/ede/simple.el
*** emacs-23.3-old/lisp/cedet/ede/simple.el	2011-01-09 01:45:14.000000000 +0800
--- emacs-23.3/lisp/cedet/ede/simple.el	2012-01-09 13:17:20.010502312 +0800
***************
*** 50,56 ****
  	      :name "Simple" :file 'ede/simple
  	      :proj-file 'ede-simple-projectfile-for-dir
  	      :load-type 'ede-simple-load
! 	      :class-sym 'ede-simple-project)
  	     t)
  
  (defcustom ede-simple-save-directory "~/.ede"
--- 50,57 ----
  	      :name "Simple" :file 'ede/simple
  	      :proj-file 'ede-simple-projectfile-for-dir
  	      :load-type 'ede-simple-load
! 	      :class-sym 'ede-simple-project
! 	      :safe-p nil)
  	     t)
  
  (defcustom ede-simple-save-directory "~/.ede"
diff -r -c emacs-23.3-old/lisp/cedet/ede.el emacs-23.3/lisp/cedet/ede.el
*** emacs-23.3-old/lisp/cedet/ede.el	2011-01-09 01:45:14.000000000 +0800
--- emacs-23.3/lisp/cedet/ede.el	2012-01-09 13:24:44.854503349 +0800
***************
*** 94,99 ****
--- 94,135 ----
    :group 'ede
    :type 'sexp) ; make this be a list of options some day
  
+ (defcustom ede-project-directories nil
+   "Directories in which EDE may search for project files.
+ If the value is t, EDE may search in any directory.
+ 
+ If the value is a function, EDE calls that function with one
+ argument, the directory name; the function should return t iff
+ EDE should look for project files in the directory.
+ 
+ Otherwise, the value should be a list of fully-expanded directory
+ names.  EDE searches for project files only in those directories.
+ If you invoke the commands \\[ede] or \\[ede-new] on a directory
+ that is not listed, Emacs will offer to add it to the list.
+ 
+ Any other value disables searching for EDE project files."
+   :group 'ede
+   :type '(choice (const :tag "Any directory" t)
+ 		 (repeat :tag "List of directories"
+ 			 (directory))
+ 		 (function :tag "Predicate"))
+   :version "23.4"
+   :risky t)
+ 
+ (defun ede-directory-safe-p (dir)
+   "Return non-nil if DIR is a safe directory to load projects from.
+ Projects that do not load a project definition as Emacs Lisp code
+ are safe, and can be loaded automatically.  Other project types,
+ such as those created with Project.ede files, are safe only if
+ specified by `ede-project-directories'."
+   (setq dir (directory-file-name (expand-file-name dir)))
+   ;; Load only if allowed by `ede-project-directories'.
+   (or (eq ede-project-directories t)
+       (and (functionp ede-project-directories)
+ 	   (funcall ede-project-directories dir))
+       (and (listp ede-project-directories)
+ 	   (member dir ede-project-directories))))
+ 
  \f
  ;;; Management variables
  
***************
*** 419,442 ****
  Sets buffer local variables for EDE."
    (let* ((ROOT nil)
  	 (proj (ede-directory-get-open-project default-directory
! 					       'ROOT)))
      (when (or proj ROOT
! 	      (ede-directory-project-p default-directory t))
  
!       (when (not proj)
! 	;; @todo - this could be wasteful.
! 	(setq proj (ede-load-project-file default-directory 'ROOT)))
  
!       (setq ede-object (ede-buffer-object (current-buffer)
  					  'ede-object-project))
  
!       (setq ede-object-root-project
! 	    (or ROOT (ede-project-root ede-object-project)))
  
!       (if (and (not ede-object) ede-object-project)
! 	  (ede-auto-add-to-target))
  
!       (ede-apply-target-options))))
  
  (defun ede-reset-all-buffers (onoff)
    "Reset all the buffers due to change in EDE.
--- 455,496 ----
  Sets buffer local variables for EDE."
    (let* ((ROOT nil)
  	 (proj (ede-directory-get-open-project default-directory
! 					       'ROOT))
! 	 (projauto nil))
! 
      (when (or proj ROOT
! 	      ;; If there is no open project, look up the project
! 	      ;; autoloader to see if we should initialize.
! 	      (setq projauto (ede-directory-project-p default-directory t)))
! 
!       (when (and (not proj) projauto)
! 
! 	;; No project was loaded, but we have a project description
! 	;; object.  This means that we can check if it is a safe
! 	;; project to load before requesting it to be loaded.
! 
! 	(when (or (oref projauto safe-p)
! 		  ;; The project style is not safe, so check if it is
! 		  ;; in `ede-project-directories'.
! 		  (let ((top (ede-toplevel-project default-directory)))
! 		    (ede-directory-safe-p top)))
  
! 	  ;; The project is safe, so load it in.
! 	  (setq proj (ede-load-project-file default-directory 'ROOT))))
  
!       ;; Only initialize EDE state in this buffer if we found a project.
!       (when proj
! 
! 	(setq ede-object (ede-buffer-object (current-buffer)
  					  'ede-object-project))
  
! 	(setq ede-object-root-project
! 	      (or ROOT (ede-project-root ede-object-project)))
  
! 	(if (and (not ede-object) ede-object-project)
! 	    (ede-auto-add-to-target))
  
! 	(ede-apply-target-options)))))
  
  (defun ede-reset-all-buffers (onoff)
    "Reset all the buffers due to change in EDE.
***************
*** 555,567 ****
  \f
  ;;; Interactive method invocations
  ;;
! (defun ede (file)
!   "Start up EDE on something.
! Argument FILE is the file or directory to load a project from."
!   (interactive "fProject File: ")
!   (if (not (file-exists-p file))
!       (ede-new file)
!     (ede-load-project-file (file-name-directory file))))
  
  (defun ede-new (type &optional name)
    "Create a new project starting of project type TYPE.
--- 609,681 ----
  \f
  ;;; Interactive method invocations
  ;;
! (defun ede (dir)
!   "Start up EDE for directory DIR.
! If DIR has an existing project file, load it.
! Otherwise, create a new project for DIR."
!   (interactive
!    ;; When choosing a directory to turn on, and we see some directory here,
!    ;; provide that as the default.
!    (let* ((top (ede-toplevel-project default-directory))
! 	  (promptdflt (or top default-directory)))
!      (list (read-directory-name "Project directory: "
! 				promptdflt promptdflt t))))
!   (unless (file-directory-p dir)
!     (error "%s is not a directory" dir))
!   (when (ede-directory-get-open-project dir)
!     (error "%s already has an open project associated with it" dir))
! 
!   ;; Check if the directory has been added to the list of safe
!   ;; directories.  It can also add the directory to the safe list if
!   ;; the user chooses.
!   (if (ede-check-project-directory dir)
!       (progn
! 	;; If there is a project in DIR, load it, otherwise do
! 	;; nothing.
! 	(ede-load-project-file dir)
! 
! 	;; Check if we loaded anything on the previous line.
! 	(if (ede-current-project dir)
! 
! 	    ;; We successfully opened an existing project.  Some open
! 	    ;; buffers may also be referring to this project.
! 	    ;; Resetting all the buffers will get them to also point
! 	    ;; at this new open project.
! 	    (ede-reset-all-buffers 1)
! 
! 	  ;; ELSE
! 	  ;; There was no project, so switch to `ede-new' which is how
! 	  ;; a user can select a new kind of project to create.
! 	  (let ((default-directory (expand-file-name dir)))
! 	    (call-interactively 'ede-new))))
! 
!     ;; If the proposed directory isn't safe, then say so.
!     (error "%s is not an allowed project directory in `ede-project-directories'"
! 	   dir)))
! 
! (defun ede-check-project-directory (dir)
!   "Check if DIR should be in `ede-project-directories'.
! If it is not, try asking the user if it should be added; if so,
! add it and save `ede-project-directories' via Customize.
! Return nil iff DIR should not be in `ede-project-directories'."
!   (setq dir (directory-file-name (expand-file-name dir))) ; strip trailing /
!   (or (eq ede-project-directories t)
!       (and (functionp ede-project-directories)
! 	   (funcall ede-project-directories dir))
!       ;; If `ede-project-directories' is a list, maybe add it.
!       (when (listp ede-project-directories)
! 	(or (member dir ede-project-directories)
! 	    (when (y-or-n-p (format "`%s' is not listed in `ede-project-directories'.
! Add it to the list of allowed project directories? "
! 				    dir))
! 	      (push dir ede-project-directories)
! 	      ;; If possible, save `ede-project-directories'.
! 	      (if (or custom-file user-init-file)
! 		  (let ((coding-system-for-read nil))
! 		    (customize-save-variable
! 		     'ede-project-directories
! 		     ede-project-directories)))
! 	      t)))))
  
  (defun ede-new (type &optional name)
    "Create a new project starting of project type TYPE.
***************
*** 596,601 ****
--- 710,720 ----
      (error "Cannot create project in non-existent directory %s" default-directory))
    (when (not (file-writable-p default-directory))
      (error "No write permissions for %s" default-directory))
+   (unless (ede-check-project-directory default-directory)
+     (error "%s is not an allowed project directory in `ede-project-directories'"
+ 	   default-directory))
+   ;; Make sure the project directory is loadable in the future.
+   (ede-check-project-directory default-directory)
    ;; Create the project
    (let* ((obj (object-assoc type 'name ede-project-class-files))
  	 (nobj (let ((f (oref obj file))
***************
*** 629,634 ****
--- 748,757 ----
  	(ede-add-subproject pp nobj)
  	(ede-commit-project pp)))
      (ede-commit-project nobj))
+   ;; Once the project is created, load it again.  This used to happen
+   ;; lazily, but with project loading occurring less often and with
+   ;; security in mind, this is now the safe time to reload.
+   (ede-load-project-file default-directory)
    ;; Have the menu appear
    (setq ede-minor-mode t)
    ;; Allert the user
***************
*** 651,661 ****
  (defun ede-rescan-toplevel ()
    "Rescan all project files."
    (interactive)
!   (let ((toppath (ede-toplevel-project default-directory))
! 	(ede-deep-rescan t))
!     (project-rescan (ede-load-project-file toppath))
!     (ede-reset-all-buffers 1)
!     ))
  
  (defun ede-new-target (&rest args)
    "Create a new target specific to this type of project file.
--- 774,789 ----
  (defun ede-rescan-toplevel ()
    "Rescan all project files."
    (interactive)
!   (if (not (ede-directory-get-open-project default-directory))
!       ;; This directory isn't open.  Can't rescan.
!       (error "Attempt to rescan a project that isn't open")
! 
!     ;; Continue
!     (let ((toppath (ede-toplevel-project default-directory))
! 	  (ede-deep-rescan t))
! 
!       (project-rescan (ede-load-project-file toppath))
!       (ede-reset-all-buffers 1))))
  
  (defun ede-new-target (&rest args)
    "Create a new target specific to this type of project file.
***************
*** 891,897 ****
    ;; Do the load
    ;;(message "EDE LOAD : %S" file)
    (let* ((file dir)
! 	 (path (expand-file-name (file-name-directory file)))
  	 (pfc (ede-directory-project-p path))
  	 (toppath nil)
  	 (o nil))
--- 1019,1025 ----
    ;; Do the load
    ;;(message "EDE LOAD : %S" file)
    (let* ((file dir)
! 	 (path (file-name-as-directory (expand-file-name dir)))
  	 (pfc (ede-directory-project-p path))
  	 (toppath nil)
  	 (o nil))
***************
*** 920,932 ****
        ;; See if it's been loaded before
        (setq o (object-assoc (ede-dir-to-projectfile pfc toppath) 'file
  			    ede-projects))
!       (if (not o)
! 	  ;; If not, get it now.
! 	  (let ((ede-constructing pfc))
! 	    (setq o (funcall (oref pfc load-type) toppath))
! 	    (when (not o)
! 	      (error "Project type error: :load-type failed to create a project"))
! 	    (ede-add-project-to-global-list o)))
  
        ;; Return the found root project.
        (when rootreturn (set rootreturn o))
--- 1048,1058 ----
        ;; See if it's been loaded before
        (setq o (object-assoc (ede-dir-to-projectfile pfc toppath) 'file
  			    ede-projects))
! 
!       ;; If not open yet, load it.
!       (unless o
! 	(let ((ede-constructing pfc))
! 	  (setq o (ede-auto-load-project pfc toppath))))
  
        ;; Return the found root project.
        (when rootreturn (set rootreturn o))
***************
*** 980,992 ****
  	     (and root
  		  (ede-find-subproject-for-directory root updir))
  	     ;; Try the all structure based search.
! 	     (ede-directory-get-open-project updir)
! 	     ;; Load up the project file as a last resort.
! 	     ;; Last resort since it uses file-truename, and other
! 	     ;; slow features.
! 	     (and (ede-directory-project-p updir)
! 		  (ede-load-project-file
! 		   (file-name-as-directory updir))))))))))
  
  (defun ede-current-project (&optional dir)
    "Return the current project file.
--- 1106,1112 ----
  	     (and root
  		  (ede-find-subproject-for-directory root updir))
  	     ;; Try the all structure based search.
! 	     (ede-directory-get-open-project updir))))))))
  
  (defun ede-current-project (&optional dir)
    "Return the current project file.
***************
*** 1000,1010 ****
      ;; No current project.
      (when (not ans)
        (let* ((ldir (or dir default-directory)))
! 	(setq ans (ede-directory-get-open-project ldir))
! 	(or ans
! 	    ;; No open project, if this dir pass project-p, then load.
! 	    (when (ede-directory-project-p ldir)
! 	      (setq ans (ede-load-project-file ldir))))))
      ;; Return what we found.
      ans))
  
--- 1120,1126 ----
      ;; No current project.
      (when (not ans)
        (let* ((ldir (or dir default-directory)))
! 	(setq ans (ede-directory-get-open-project ldir))))
      ;; Return what we found.
      ans))
  
***************
*** 1059,1070 ****
    "Return the project which is the parent of TARGET.
  It is recommended you track the project a different way as this function
  could become slow in time."
!   ;; @todo - use ede-object-project as a starting point.
!   (let ((ans nil) (projs ede-projects))
!     (while (and (not ans) projs)
!       (setq ans (ede-target-in-project-p (car projs) target)
! 	    projs (cdr projs)))
!     ans))
  
  (defmethod ede-find-target ((proj ede-project) buffer)
    "Fetch the target in PROJ belonging to BUFFER or nil."
--- 1175,1187 ----
    "Return the project which is the parent of TARGET.
  It is recommended you track the project a different way as this function
  could become slow in time."
!   (or ede-object-project
!       ;; If not cached, derive it from the current directory of the target.
!       (let ((ans nil) (projs ede-projects))
! 	(while (and (not ans) projs)
! 	  (setq ans (ede-target-in-project-p (car projs) target)
! 		projs (cdr projs)))
! 	ans)))
  
  (defmethod ede-find-target ((proj ede-project) buffer)
    "Fetch the target in PROJ belonging to BUFFER or nil."

Re: Security flaw in EDE; new release plans

[Daniel Colascione]

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

On 1/8/12 10:07 PM, Chong Yidong wrote:
> A patch to fix this problem, for the Emacs 23.3 release, is attached.
> It prevents EDE from loading Project.ede files, except in directories
> explicitly designated as "safe" by the user via the new list variable
> `ede-project-directories'.  The value of this variable is initially the
> empty list; Emacs offers to add to it when the user invokes the `M-x
> ede' or `M-x ede-new' command.  EDE project types that do not use
> Project.ede (e.g. those that scan makefiles for build information) are
> unaffected, since they do not involve loading Lisp code.

It's great that this is being fixed so quickly.

> Due to this problem, we will make a 23.4 release from the emacs-23
> branch.
[snip]
> In a few days,
> I will make the 23.3.90 pretest; during this brief window, if anyone
> thinks there is another bug fix that ought to go into 23.4, please
> promptly raise the issue on emacs-devel---but we will be very
> conservative about allowing commits, in order to release 23.4 ASAP.

I never got around to committing the patch below to the emacs-23
branch. Would it be okay to add it before the 23.4 release?

*** /a/simple.el	2012-01-08 22:29:04.904878400 -0800
--- /b/simple.el	2012-01-08 22:29:18.867504900 -0800
***************
*** 6660,6665 ****
--- 6660,6667 ----
               (display-warning package (nth 3 list) :warning)))
      (error nil)))

+ (put 'lexical-binding 'safe-local-variable t)
+
  (mapc (lambda (elem)
          (eval-after-load (car elem) `(bad-package-check ',(car elem))))
        bad-packages-alist)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

Re: Security flaw in EDE; new release plans

[Chong Yidong]

Daniel Colascione <dancol@dancol.org> writes:

> I never got around to committing the patch below to the emacs-23
> branch. Would it be okay to add it before the 23.4 release?
>
> + (put 'lexical-binding 'safe-local-variable t)

What's the rationale?  If Emacs 23 users try to load Lisp libraries that
use lexical binding, that will tend to lead to bugs, so why make it
*easier* for that to happen?

Re: Security flaw in EDE; new release plans

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 09 Jan 2012 14:07:47 +0800, Chong Yidong <cyd@gnu.org> said:

> In a few days, I will make the 23.3.90 pretest; during this brief
> window, if anyone thinks there is another bug fix that ought to go
> into 23.4, please promptly raise the issue on emacs-devel---but we
> will be very conservative about allowing commits, in order to
> release 23.4 ASAP.  If no serious problems are found with the
> 23.3.90 pretest, the 23.4 release will probably follow right after.

The latest Emacs 23 Mac port contains some backported patches for
redisplay bugs that was found in the trunk (revno 106534,
106517(Bug#10119), 106357, 106345(Bug#9496), 106279(Bug#9947), 106223,
and 106220).  They can be used as candidates.  Among them, I think at
least revno 106534 and 106357 should be backported to the emacs-23
branch because otherwise users may occasionally see very strange
results:

  http://lists.gnu.org/archive/html/emacs-devel/2011-11/msg00278.html

This strange display could not be observed with Emacs 23.3 so easily
except the very rare cases, because this bug has emerged by a fix for
another bug:

  http://lists.gnu.org/archive/html/emacs-devel/2011-11/msg00308.html

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Security flaw in EDE; new release plans

[Daniel Colascione]

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

On 1/8/12 11:06 PM, Chong Yidong wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
>> I never got around to committing the patch below to the emacs-23
>> branch. Would it be okay to add it before the 23.4 release?
>>
>> + (put 'lexical-binding 'safe-local-variable t)
> 
> What's the rationale?  If Emacs 23 users try to load Lisp libraries that
> use lexical binding, that will tend to lead to bugs, so why make it
> *easier* for that to happen?

My proposed patch makes it painless to edit Emacs 24 lisp using Emacs
23. A user might want to read or backport Emacs 24 lisp files, and
because it's possible to write lisp that works correctly whether
lexical-binding is on or off, a user might even legitimately want to
load these files.

The warning about the lexical-binding variable appears only when a
user tries to edit a file with lexical-binding. If an Emacs 23 user
tries to load or compile such a file, he won't receive a warning. If
we wants to guard against loading a file in an Emacs without support
for lexical-binding, an (assert (boundp 'lexical-binding)) at toplevel
should do the trick; a more general solution would be to add code to
the Emacs 23 lisp reader or byte compiler to reject files that specify
lexical-binding.

As far as editing itself is concerned, though, lexical-binding is
indeed a harmless variable in Emacs 23: inert, sure, but harmless.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

Re: Security flaw in EDE; new release plans

[Jan D.]

YAMAMOTO Mitsuharu skrev 2012-01-09 08:16:
>>>>>> On Mon, 09 Jan 2012 14:07:47 +0800, Chong Yidong<cyd@gnu.org>  said:
>
>> In a few days, I will make the 23.3.90 pretest; during this brief
>> window, if anyone thinks there is another bug fix that ought to go
>> into 23.4, please promptly raise the issue on emacs-devel---but we
>> will be very conservative about allowing commits, in order to
>> release 23.4 ASAP.  If no serious problems are found with the
>> 23.3.90 pretest, the 23.4 release will probably follow right after.
>
> The latest Emacs 23 Mac port contains some backported patches for
> redisplay bugs that was found in the trunk (revno 106534,
> 106517(Bug#10119), 106357, 106345(Bug#9496), 106279(Bug#9947), 106223,
> and 106220).  They can be used as candidates.  Among them, I think at
> least revno 106534 and 106357 should be backported to the emacs-23
> branch because otherwise users may occasionally see very strange
> results:
>

If we are fixing NS-stuff, the fact that Emacs 23 does not resize 
correctly under OSX Lion must be a candidate.

	Jan D.

Re: Security flaw in EDE; new release plans

[Ulrich Mueller]

>>>>> On Mon, 09 Jan 2012, Chong Yidong wrote:

> Hiroshi Oota has found a security flaw in EDE (part of CEDET),
> a development tool included in Emacs.  [...]

Does this also affect the standalone version of CEDET, i.e. cedet-1.0
from <http://cedet.sourceforge.net/>?

Re: Security flaw in EDE; new release plans

[Chong Yidong]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> The latest Emacs 23 Mac port contains some backported patches for
> redisplay bugs that was found in the trunk (revno 106534,
> 106517(Bug#10119), 106357, 106345(Bug#9496), 106279(Bug#9947), 106223,
> and 106220).  They can be used as candidates.  Among them, I think at
> least revno 106534 and 106357 should be backported to the emacs-23
> branch because otherwise users may occasionally see very strange
> results

Thanks.  It's certainly prudent to fix all those unitialized variable
bugs.  Committed except for 106517, which I'm going to stare at for a
bit longer.

Re: Security flaw in EDE; new release plans

[Chong Yidong]

Ulrich Mueller <ulm@gentoo.org> writes:

> Does this also affect the standalone version of CEDET, i.e. cedet-1.0
> from <http://cedet.sourceforge.net/>?

Yes.

Re: Security flaw in EDE; new release plans

[Chong Yidong]

"Jan D." <jan.h.d@swipnet.se> writes:

> If we are fixing NS-stuff, the fact that Emacs 23 does not resize
> correctly under OSX Lion must be a candidate.

The bugs that YM pointed out were not NS-specific.

Re: Security flaw in EDE; new release plans

[Ulrich Mueller]

>>>>> On Mon, 09 Jan 2012, Chong Yidong wrote:

>> Does this also affect the standalone version of CEDET, i.e. cedet-1.0
>> from <http://cedet.sourceforge.net/>?

> Yes.

The patch for Emacs 23 fails for it, unfortunately. In case anyone is
interested, a (almost trivially) backported version of the patch can
be found here:
<http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/app-emacs/cedet/files/cedet-1.0-ede_security_fix.patch>

Re: Security flaw in EDE; new release plans

[Stefan Monnier]

> + (put 'lexical-binding 'safe-local-variable t)

I'm fine with the concept, but please do it with the same code as used
in Emacs-24 (or at least at the same place), so that merging the
emacs-23 branch into trunk won't bring a duplicate setting.
Take a look at files.el on the trunk where we do that.


        Stefan

Re: Security flaw in EDE; new release plans

[David Engster]

Ulrich Mueller writes:
>>>>>> On Mon, 09 Jan 2012, Chong Yidong wrote:
>
>>> Does this also affect the standalone version of CEDET, i.e. cedet-1.0
>>> from <http://cedet.sourceforge.net/>?
>
>> Yes.
>
> The patch for Emacs 23 fails for it, unfortunately. In case anyone is
> interested, a (almost trivially) backported version of the patch can
> be found here:
> <http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/app-emacs/cedet/files/cedet-1.0-ede_security_fix.patch>

A new CEDET 1.0.1 (or similar) will be released soon. The bzr repos
trunk and newtrunk were updated.

-David

Re: Security flaw in EDE; new release plans

[Glenn Morris]

Chong Yidong wrote:

> In a few days, I will make the 23.3.90 pretest; during this brief
> window, if anyone thinks there is another bug fix that ought to go
> into 23.4, please promptly raise the issue on emacs-devel---but we
> will be very conservative about allowing commits, in order to release
> 23.4 ASAP.

You might consider fixing

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8497

which could probably be done by just adding "i386" to cpp_undefs in
configure.in. The only thing I am aware of that affecting is
src/m/amdx86-64.h, when someone tries to compile a 32-bit exe on a
64-bit arch without specifying an explicit --build argument, but that
does not seem important.


And also allowing configure's --with-crt-dir argument to apply to all
platforms, not just x86-64 and s390x (eg for Debian multiarch).
Perhaps this is too invasive though.

Re: Security flaw in EDE; new release plans

[Chong Yidong]

Glenn Morris <rgm@gnu.org> writes:

> You might consider fixing
>
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8497
>
> which could probably be done by just adding "i386" to cpp_undefs in
> configure.in. The only thing I am aware of that affecting is
> src/m/amdx86-64.h, when someone tries to compile a 32-bit exe on a
> 64-bit arch without specifying an explicit --build argument, but that
> does not seem important.

This sounds desirable, but the discussion in that bug is not as concrete
as I'd like.  What's the specific patch you're taking about?

BTW, would you mind updating the copyright years on the emacs-23 branch?
Thanks.  (No need to worry about merging; we won't be doing any emacs-23
to trunk merges anymore, only cherry-picking fixes from trunk to
emacs-23)

Re: Security flaw in EDE; new release plans

[Sven Joachim]

The following patch (committed on the trunk in revision 100042) is
necessary for the "--with-crt-dir=" configure option to actually work
and let Emacs build on current Debian/Ubuntu systems:

--8<---------------cut here---------------start------------->8---
=== modified file 'src/s/gnu-linux.h'
--- src/s/gnu-linux.h	2011-01-02 23:50:46 +0000
+++ src/s/gnu-linux.h	2011-07-27 14:36:13 +0000
@@ -168,7 +168,7 @@ along with GNU Emacs.  If not, see <http
 /* Ask GCC where to find libgcc.a.  */
 #define LIB_GCC `$(CC) $(C_SWITCH_X_SITE) -print-libgcc-file-name`
 
-#define START_FILES pre-crt0.o /usr/lib/crt1.o /usr/lib/crti.o
+#define START_FILES pre-crt0.o $(CRT_DIR)/crt1.o $(CRT_DIR)/crti.o
 
 /* Here is how to find X Windows.  LD_SWITCH_X_SITE_AUX gives an -R option
    says where to find X windows at run time.  */
@@ -198,7 +198,7 @@ along with GNU Emacs.  If not, see <http
 #define LIBS_DEBUG
 #undef LIB_GCC
 #define LIB_GCC
-#define LIB_STANDARD -lgcc -lc -lgcc /usr/lib/crtn.o
+#define LIB_STANDARD -lgcc -lc -lgcc $(CRT_DIR)/crtn.o
 
 /* Don't use -g in test compiles in configure.
    This is so we will use the same shared libs for that linking

--8<---------------cut here---------------end--------------->8---

See http://bugs.debian.org/629567 for details.

Cheers,
       Sven

Re: Security flaw in EDE; new release plans

[Stefan Monnier]

> Thanks.  (No need to worry about merging; we won't be doing any emacs-23
> to trunk merges anymore, only cherry-picking fixes from trunk to
> emacs-23)

I don't think it costs much to "worry about merging", so better do it,
just in case.


        Stefan

Re: Security flaw in EDE; new release plans

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 09 Jan 2012 17:52:45 +0800, Chong Yidong <cyd@gnu.org> said:

> YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
>> The latest Emacs 23 Mac port contains some backported patches for
>> redisplay bugs that was found in the trunk (revno 106534,
>> 106517(Bug#10119), 106357, 106345(Bug#9496), 106279(Bug#9947),
>> 106223, and 106220).  They can be used as candidates.  Among them,
>> I think at least revno 106534 and 106357 should be backported to
>> the emacs-23 branch because otherwise users may occasionally see
>> very strange results

> Thanks.  It's certainly prudent to fix all those unitialized
> variable bugs.  Committed except for 106517, which I'm going to
> stare at for a bit longer.

I'd give revno 106677 (Bug#8992, wrong mouse highlight background
ascent and height in the `xft' font backend) as another candidate.
This is not a critical bug, but looks visually odd and the fix is safe
to install (the drawing functions in the other font backend drivers
already do so).

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Security flaw in EDE; new release plans

[Glenn Morris]

Chong Yidong wrote:

> This sounds desirable, but the discussion in that bug is not as concrete
> as I'd like.  What's the specific patch you're taking about?

(untested)

*** configure.in	2012-01-11 07:52:35 +0000
--- configure.in	2012-01-11 08:06:35 +0000
***************
*** 3103,3109 ****
  # the C preprocessor to some helpful value like 1, or maybe the empty
  # string.  Needless to say consequent macro substitutions are less
  # than conducive to the makefile finding the correct directory.
! [cpp_undefs="`echo $srcdir $configuration $canonical unix |
    sed -e 's/[^a-zA-Z0-9_]/ /g' -e 's/^/ /' -e 's/  *$//' \
    -e 's/  */ -U/g' -e 's/-U[0-9][^ ]*//g'`"]
  
--- 3103,3116 ----
  # the C preprocessor to some helpful value like 1, or maybe the empty
  # string.  Needless to say consequent macro substitutions are less
  # than conducive to the makefile finding the correct directory.
! 
! [
! case $canonical in
!      i[456]86-*) extra_undefs=i386 ;;
!      *) extra_undefs= ;;
! esac
! 
! cpp_undefs="`echo $srcdir $configuration $canonical unix $extra_undefs |
    sed -e 's/[^a-zA-Z0-9_]/ /g' -e 's/^/ /' -e 's/  *$//' \
    -e 's/  */ -U/g' -e 's/-U[0-9][^ ]*//g'`"]
  

> BTW, would you mind updating the copyright years on the emacs-23 branch?

:(