Re: "Local variables" denial-of-service attack

[Ulrich Mueller]

>>>>> On Fri, 09 Jan 2009, Stefan Monnier wrote:

>> Local Variab*les:
>> byte-compile-warnings: #1=("circular" "object" . #1#)
>> End:

>> Visiting such a file will result in Emacs hanging and consuming all
>> CPU time. (Normally, aborting with C-g is possible. However, if the
>> file is opened with emacsclient there seems to be no way to recover.)

> Hmm... that's not good.  It's not the end of the world since it's "only"
> a DoS, but we should try and fix it.  I see two ways:
> 1 - don't allow (in file-local vars and .dir-local.el) the # escapes
>     that permit creation of those circular objects.
> 2 - be more careful about circularities in the
>     safe-local-variable predicates.

> Nb 1 is easier and will solve it "for all cases", so it's probably
> a better choice.

Coming back to this, the following patch works for me:

etc/NEWS entry:

** Reading of circular structures in file-local and directory-local
variable lists has been disabled.

lisp/ChangeLog entry:

2009-04-02  Ulrich Mueller  <address@hidden>

        * files.el (hack-local-variables-prop-line)
        (hack-local-variables, dir-locals-read-from-file):
        Bind read-circle to nil, in order to disable reading of recursive
        structures in file-local and dir-local variables.

src/ChangeLog entry:

2009-04-02  Ulrich Mueller  <address@hidden>

        * lread.c (read_circle): New variable.
        (syms_of_lread): DEFVAR_BOOL read-circle, initialize to t.
        (read1): Disable #N= and #N# syntax for recursive structures if
        read-circle is nil.


--- emacs/lisp/files.el~
+++ emacs/lisp/files.el
@@ -2896,10 +2896,11 @@
               ;; There used to be a downcase here,
               ;; but the manual didn't say so,
               ;; and people want to set var names that aren't all lc.
-              (let ((key (intern (match-string 1)))
-                    (val (save-restriction
-                           (narrow-to-region (point) end)
-                           (read (current-buffer)))))
+              (let* ((read-circle nil)
+                     (key (intern (match-string 1)))
+                     (val (save-restriction
+                            (narrow-to-region (point) end)
+                            (read (current-buffer)))))
                 ;; It is traditional to ignore
                 ;; case when checking for `mode' in set-auto-mode,
                 ;; so we must do that here as well.
@@ -3044,7 +3045,9 @@
                  (skip-chars-forward "^:\n")
                  (if (eolp) (error "Missing colon in local variables entry"))
                  (skip-chars-backward " \t")
-                 (let* ((str (buffer-substring beg (point)))
+                 ;; Bind `read-circle' to nil to disable recursive structures.
+                 (let* ((read-circle nil)
+                        (str (buffer-substring beg (point)))
                         (var (read str))
                         val)
                    ;; Read the variable value.
@@ -3323,7 +3326,8 @@
     ;; We should probably store the modtime of FILE and then
     ;; reload it whenever it changes.
     (insert-file-contents file)
-    (let* ((dir-name (file-name-directory file))
+    (let* ((read-circle nil)
+          (dir-name (file-name-directory file))
           (class-name (intern dir-name))
           (variables (read (current-buffer))))
       (dir-locals-set-class-variables class-name variables)
--- emacs/src/lread.c~
+++ emacs/src/lread.c
@@ -156,6 +156,9 @@
 /* List of (SYMBOL . POSITION) accumulated so far. */
 Lisp_Object Vread_symbol_positions_list;
 
+/* Nonzero means allow #N= and #N# syntax for recursive structures. */
+static int read_circle;
+
 /* List of descriptors now open for Fload.  */
 static Lisp_Object load_descriptor_list;
 
@@ -2558,7 +2561,7 @@
              c = READCHAR;
            }
          /* #n=object returns object, but associates it with n for #n#.  */
-         if (c == '=')
+         if (c == '=' && read_circle)
            {
              /* Make a placeholder for #n# to use temporarily */
              Lisp_Object placeholder;
@@ -2580,7 +2583,7 @@
              return tem;
            }
          /* #n# returns a previously read object.  */
-         if (c == '#')
+         if (c == '#' && read_circle)
            {
              tem = Fassq (make_number (n), read_objects);
              if (CONSP (tem))
@@ -4215,6 +4218,12 @@
 were read in. */);
   Vread_symbol_positions_list = Qnil;
 
+  DEFVAR_BOOL ("read-circle", &read_circle,
+              doc: /* Non-nil means allow #N= and #N# syntax for recursive 
structures.
+This is normally bound by functions like `hack-local-variables' to
+disable this syntax in `read', and is not meant for users to change. */);
+  read_circle = 1;
+
   DEFVAR_LISP ("load-path", &Vload_path,
               doc: /* *List of directories to search for files to load.
 Each element is a string (directory name) or nil (try default directory).