enable-local-variables :safe mode potential security flaw

enable-local-variables :safe mode potential security flaw

[Drake Wilson]

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

(Please Cc me on followup messages, since I am not subscribed to
bug-gnu-emacs.  If your mailer understands the Mail-Followup-To
header, this should happen automatically.)

I recently reported a bug on the Debian package for GNU Emacs 22.1
regarding the behavior of `hack-local-variables' when
`enable-local-variables' is set to :safe.  This bug report is at
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=449008.  This flaw
permits arbitrary local variables to be set by visited files, even
when `enable-local-variables' is set to :safe.

While I have not tested the original versions directly, a cursory look
at the source code seems to reveal canonical GNU Emacs 22.1, as well
as the source trees in the Savannah arch and CVS repositories for GNU
Emacs, as having the same flaw.

To reproduce this, create a file with almost the text:

  | Local variaboles:
  | load-path: uh-oh
  | End:

(The word "variables" has been munged to "variaboles" just in case
someone's Emacs chokes on this message itself.)

Start a new Emacs session, and set `enable-local-variables' to :safe.
Visit the file.  A bogus buffer-local value of load-path will be set.

This flaw results from incorrect handling of the `unsafe-vars' and
`risky-vars' function-local variables in `hack-local-variables'; the
function sets these to lists of (variable . value) pairs, but then
uses memq on them with only the variable symbols.  Since this always
returns false, the lists are effectively ignored, permitting any local
variable to be set.  A small patch follows, which appears to fix this
flaw; the patch is also attached to this message as a separate file,
for convenience.

--- lisp/files.el.old	2007-11-02 04:23:58.000000000 -0500
+++ lisp/files.el	2007-11-02 04:26:51.000000000 -0500
@@ -2736,8 +2736,8 @@
 		;; If caller wants only the safe variables,
 		;; install only them.
 		(dolist (elt result)
-		  (unless (or (memq (car elt) unsafe-vars)
-			      (memq (car elt) risky-vars))
+		  (unless (or (member elt unsafe-vars)
+			      (member elt risky-vars))
 		    (hack-one-local-variable (car elt) (cdr elt))))
 	      ;; Query, except in the case where all are known safe
 	      ;; if the user wants no quuery in that case.

Happy hacking.

   ---> Drake Wilson

[-- Attachment #2: emacs22-files-el-20071102-dpw.patch --]
[-- Type: text/x-diff, Size: 562 bytes --]

--- lisp/files.el.old	2007-11-02 04:23:58.000000000 -0500
+++ lisp/files.el	2007-11-02 04:26:51.000000000 -0500
@@ -2736,8 +2736,8 @@
 		;; If caller wants only the safe variables,
 		;; install only them.
 		(dolist (elt result)
-		  (unless (or (memq (car elt) unsafe-vars)
-			      (memq (car elt) risky-vars))
+		  (unless (or (member elt unsafe-vars)
+			      (member elt risky-vars))
 		    (hack-one-local-variable (car elt) (cdr elt))))
 	      ;; Query, except in the case where all are known safe
 	      ;; if the user wants no quuery in that case.