[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Security flaw in EDE; new release plans
|
From: |
Chong Yidong |
|
Subject: |
Security flaw in EDE; new release plans |
|
Date: |
Mon, 09 Jan 2012 14:07:47 +0800 |
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.
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))))
+
;;; 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 ****
;;; 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 ----
;;; 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."
- Security flaw in EDE; new release plans,
Chong Yidong <=