[Emacs-diffs] /srv/bzr/emacs/trunk r106858: Fix EDE security flaw involving loading arbitrary Lisp from Project.ede.

[Chong Yidong]

------------------------------------------------------------
revno: 106858 [merge]
author: Eric Ludlam <address@hidden>
committer: Chong Yidong <address@hidden>
branch nick: trunk
timestamp: Fri 2012-01-13 21:19:25 +0800
message:
  Fix EDE security flaw involving loading arbitrary Lisp from Project.ede.
  
  * lisp/ede.el (ede-project-directories): New option.
  (ede-directory-safe-p): Check it.
  (ede-initialize-state-current-buffer, ede, ede-new)
  (ede-check-project-directory, ede-rescan-toplevel)
  (ede-load-project-file, ede-parent-project, ede-current-project):
  (ede-target-parent): Avoid loading in a project unless it is safe,
  since it may involve malicious code.  This security flaw was
  pointed out by Hiroshi Oota.
  
  * lisp/ede/auto.el (ede-project-autoload): Add safe-p slot.
  (ede-project-class-files): Projects using Project.ede are unsafe.
  (ede-auto-load-project): New method.
  
  * lisp/ede/simple.el (ede-project-class-files): Mark as unsafe.
modified:
  lisp/cedet/ChangeLog
  lisp/cedet/ede.el
  lisp/cedet/ede/auto.el
  lisp/cedet/ede/simple.el

=== modified file 'lisp/cedet/ChangeLog'
--- a/lisp/cedet/ChangeLog      2012-01-05 09:46:05 +0000
+++ b/lisp/cedet/ChangeLog      2012-01-13 13:19:25 +0000
@@ -1,3 +1,20 @@
+2012-01-09  Eric Ludlam  <address@hidden>
+
+       * ede.el (ede-project-directories): New option.
+       (ede-directory-safe-p): Check it.
+       (ede-initialize-state-current-buffer, ede, ede-new)
+       (ede-check-project-directory, ede-rescan-toplevel)
+       (ede-load-project-file, ede-parent-project, ede-current-project):
+       (ede-target-parent): Avoid loading in a project unless it is safe,
+       since it may involve malicious code.  This security flaw was
+       pointed out by Hiroshi Oota.
+
+       * ede/auto.el (ede-project-autoload): Add safe-p slot.
+       (ede-project-class-files): Projects using Project.ede are unsafe.
+       (ede-auto-load-project): New method.
+
+       * ede/simple.el (ede-project-class-files): Mark as unsafe.
+
 2011-12-19  Sam Steingold  <address@hidden>
 
        * semantic/edit.el (semantic-edits-incremental-parser): Add the

=== modified file 'lisp/cedet/ede.el'
--- a/lisp/cedet/ede.el 2012-01-05 09:46:05 +0000
+++ b/lisp/cedet/ede.el 2012-01-13 13:19:25 +0000
@@ -94,6 +94,42 @@
   :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
 
@@ -420,24 +456,42 @@
 Sets buffer local variables for EDE."
   (let* ((ROOT nil)
         (proj (ede-directory-get-open-project default-directory
-                                              'ROOT)))
+                                              'ROOT))
+        (projauto nil))
+
     (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)
+             ;; 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))))
+       (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.
@@ -557,13 +611,73 @@
 
 ;;; 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 (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 from project type TYPE.
@@ -598,6 +712,11 @@
     (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))
@@ -631,6 +750,10 @@
        (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
@@ -653,11 +776,16 @@
 (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)
-    ))
+  (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.
@@ -893,7 +1021,7 @@
   ;; Do the load
   ;;(message "EDE LOAD : %S" file)
   (let* ((file dir)
-        (path (expand-file-name (file-name-directory file)))
+        (path (file-name-as-directory (expand-file-name dir)))
         (pfc (ede-directory-project-p path))
         (toppath nil)
         (o nil))
@@ -922,13 +1050,11 @@
       ;; 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)))
+
+      ;; 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))
@@ -982,13 +1108,7 @@
             (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))))))))))
+            (ede-directory-get-open-project updir))))))))
 
 (defun ede-current-project (&optional dir)
   "Return the current project file.
@@ -1002,11 +1122,7 @@
     ;; 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))))))
+       (setq ans (ede-directory-get-open-project ldir))))
     ;; Return what we found.
     ans))
 
@@ -1061,12 +1177,13 @@
   "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))
+  (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."

=== modified file 'lisp/cedet/ede/auto.el'
--- a/lisp/cedet/ede/auto.el    2012-01-05 09:46:05 +0000
+++ b/lisp/cedet/ede/auto.el    2012-01-13 13:19:25 +0000
@@ -58,6 +58,13 @@
          :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,13 +76,15 @@
                         :name "Make" :file 'ede/proj
                         :proj-file "Project.ede"
                         :load-type 'ede-proj-load
-                        :class-sym 'ede-proj-project)
+                        :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)
+                        :class-sym 'ede-proj-project
+                        :safe-p nil)
    (ede-project-autoload "automake"
                         :name "automake" :file 'ede/project-am
                         :proj-file "Makefile.am"
@@ -84,6 +93,8 @@
                         :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,6 +133,19 @@
     (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)
 

=== modified file 'lisp/cedet/ede/simple.el'
--- a/lisp/cedet/ede/simple.el  2012-01-05 09:46:05 +0000
+++ b/lisp/cedet/ede/simple.el  2012-01-13 13:19:25 +0000
@@ -50,7 +50,8 @@
              :name "Simple" :file 'ede/simple
              :proj-file 'ede-simple-projectfile-for-dir
              :load-type 'ede-simple-load
-             :class-sym 'ede-simple-project)
+             :class-sym 'ede-simple-project
+             :safe-p nil)
             t)
 
 (defcustom ede-simple-save-directory "~/.ede"