emacs-30 b5158bd1914: elisp-mode.el: Disable Flymake byte-compile backend in untrusted files

[Stefan Monnier]

branch: emacs-30
commit b5158bd191422e46273c4d9412f2bf097e2da2e0
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>

    elisp-mode.el: Disable Flymake byte-compile backend in untrusted files
    
    To address serious security issues (CVE-2024-53920), disable
    `elisp-flymake-byte-compile` except in those files explicitly
    specified as "trusted".
    
    For that introduce a new custom var `trusted-files` and new
    function `trusted-content-p`.
    
    While at it, similarly skip the implicit macroexpansion done during
    completion if the current file is not trusted.
    
    * lisp/files.el (trusted-files): New variable.
    (trusted-content-p): New function.
    
    * lisp/progmodes/elisp-mode.el (elisp--safe-macroexpand-all):
    New function, extracted from `elisp--local-variables`.
    Use `trusted-content-p`.
    (elisp--local-variables): Use it.
    (elisp-flymake-byte-compile): Disable according to `trusted-content-p`.
---
 etc/NEWS                     | 14 +++++++++++
 lisp/files.el                | 49 +++++++++++++++++++++++++++++++++++++++
 lisp/progmodes/elisp-mode.el | 55 ++++++++++++++++++++++++++++++--------------
 3 files changed, 101 insertions(+), 17 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 3d691cfac40..a9c8f6c4801 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -199,6 +199,14 @@ see the variable 'url-request-extra-headers'.
 
 * Changes in Emacs 30.1
 
+** New variable 'trusted-files' to allow potentially dangerous features.
+This variable lists those files and directories whose content Emacs should
+consider as sufficiently trusted to run any part of the code contained
+therein even without any explicit user request.
+For example, Flymake's backend for Emacs Lisp consults this variable
+and disables itself with an "untrusted content" warning if the file
+is not listed.
+
 ---
 ** Emacs now supports Unicode Standard version 15.1.
 
@@ -1859,6 +1867,12 @@ In the past they included a terminating newline in most 
cases but not all.
 
 ** Emacs Lisp mode
 
+*** 'elisp-flymake-byte-compile' is disabled for untrusted files.
+For security reasons, this backend can be used only in those files
+specified as trusted according to 'trusted-files' and emits an
+"untrusted content" warning otherwise.
+This fixes CVE-2024-53920.
+
 ---
 *** ',@' now has 'prefix' syntax.
 Previously, the '@' character, which normally has 'symbol' syntax,
diff --git a/lisp/files.el b/lisp/files.el
index 63a08ce5b22..5d9e6440f5b 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -714,6 +714,55 @@ buffer contents as untrusted.
 This variable might be subject to change without notice.")
 (put 'untrusted-content 'permanent-local t)
 
+(defcustom trusted-files nil
+  "List of files and directories whose content we trust.
+Be extra careful here since trusting means that Emacs might execute the
+code contained within those files and directories without an explicit
+request by the user.
+One important case when this might happen is when `flymake-mode' is
+enabled (for example, when it is added to a mode hook).
+Each element of the list should be a string:
+- If it ends in \"/\", it is considered as a directory name and means that
+  Emacs should trust all the files whose name has this directory as a prefix.
+- else it is considered as a file name.
+Use abbreviated file names.  For example, an entry \"~/mycode\" means
+that Emacs will trust all the files in your directory \"mycode\".
+This variable can also be set to `:all', in which case Emacs will trust
+all files, which opens a gaping security hole."
+  :type '(choice (repeat :tag "List" file)
+                 (const :tag "Trust everything (DANGEROUS!)" :all))
+  :version "30.1")
+(put 'trusted-files 'risky-local-variable t)
+
+(defun trusted-content-p ()
+  "Return non-nil if we trust the contents of the current buffer.
+Here, \"trust\" means that we are willing to run code found inside of it.
+See also `trusted-files'."
+  ;; We compare with `buffer-file-truename' i.s.o `buffer-file-name'
+  ;; to try and avoid marking as trusted a file that's merely accessed
+  ;; via a symlink that happens to be inside a trusted dir.
+  (and (not untrusted-content)
+       buffer-file-truename
+       (with-demoted-errors "trusted-content-p: %S"
+         (let ((exists (file-exists-p buffer-file-truename)))
+           (or
+            (eq trusted-files :all)
+            ;; We can't avoid trusting the user's init file.
+            (if (and exists user-init-file)
+                (file-equal-p buffer-file-truename user-init-file)
+              (equal buffer-file-truename user-init-file))
+            (let ((file (abbreviate-file-name buffer-file-truename))
+                  (trusted nil))
+              (dolist (tf trusted-files)
+                (when (or (if exists (file-equal-p tf file) (equal tf file))
+                          ;; We don't use `file-in-directory-p' here, because
+                          ;; we want to err on the conservative side: "guilty
+                          ;; until proven innocent".
+                          (and (string-suffix-p "/" tf)
+                               (string-prefix-p tf file)))
+                  (setq trusted t)))
+              trusted))))))
+
 ;; This is an odd variable IMO.
 ;; You might wonder why it is needed, when we could just do:
 ;; (setq-local enable-local-variables nil)
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 9bf6f9217c8..ab79082d1b2 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -448,6 +448,34 @@ be used instead.
 This is used to try and avoid the most egregious problems linked to the
 use of `macroexpand-all' as a way to find the \"underlying raw code\".")
 
+(defvar elisp--macroexpand-untrusted-warning t)
+
+(defun elisp--safe-macroexpand-all (sexp)
+  (if (not (trusted-content-p))
+      ;; FIXME: We should try and do better here, either using a notion
+      ;; of "safe" macros, or with `bwrap', or ...
+      (progn
+        (when elisp--macroexpand-untrusted-warning
+          (setq-local elisp--macroexpand-untrusted-warning nil) ;Don't spam!
+          (message "Completion of local vars is disabled in %s (untrusted 
content)"
+                   (buffer-name)))
+        sexp)
+    (let ((macroexpand-advice
+           (lambda (expander form &rest args)
+             (condition-case err
+                 (apply expander form args)
+               (error
+                (message "Ignoring macroexpansion error: %S" err) form)))))
+      (unwind-protect
+          ;; Silence any macro expansion errors when
+          ;; attempting completion at point (bug#58148).
+          (let ((inhibit-message t)
+                (macroexp-inhibit-compiler-macros t)
+                (warning-minimum-log-level :emergency))
+            (advice-add 'macroexpand-1 :around macroexpand-advice)
+            (macroexpand-all sexp elisp--local-macroenv))
+        (advice-remove 'macroexpand-1 macroexpand-advice)))))
+
 (defun elisp--local-variables ()
   "Return a list of locally let-bound variables at point."
   (save-excursion
@@ -463,23 +491,8 @@ use of `macroexpand-all' as a way to find the \"underlying 
raw code\".")
                        (car (read-from-string
                              (concat txt "elisp--witness--lisp" closer)))
                      ((invalid-read-syntax end-of-file) nil)))
-             (macroexpand-advice
-              (lambda (expander form &rest args)
-                (condition-case err
-                    (apply expander form args)
-                  (error
-                   (message "Ignoring macroexpansion error: %S" err) form))))
-             (sexp
-              (unwind-protect
-                  ;; Silence any macro expansion errors when
-                  ;; attempting completion at point (bug#58148).
-                  (let ((inhibit-message t)
-                        (macroexp-inhibit-compiler-macros t)
-                        (warning-minimum-log-level :emergency))
-                    (advice-add 'macroexpand-1 :around macroexpand-advice)
-                    (macroexpand-all sexp elisp--local-macroenv))
-                (advice-remove 'macroexpand-1 macroexpand-advice)))
-             (vars (elisp--local-variables-1 nil sexp)))
+             (vars (elisp--local-variables-1
+                    nil (elisp--safe-macroexpand-all sexp))))
         (delq nil
               (mapcar (lambda (var)
                         (and (symbolp var)
@@ -2188,6 +2201,14 @@ directory of the buffer being compiled, and nothing 
else.")
   "A Flymake backend for elisp byte compilation.
 Spawn an Emacs process that byte-compiles a file representing the
 current buffer state and calls REPORT-FN when done."
+  (unless (trusted-content-p)
+    ;; FIXME: Use `bwrap' and friends to compile untrusted content.
+    ;; FIXME: We emit a message *and* signal an error, because by default
+    ;; Flymake doesn't display the warning it puts into "*flmake log*".
+    (message "Disabling elisp-flymake-byte-compile in %s (untrusted content)"
+             (buffer-name))
+    (error "Disabling elisp-flymake-byte-compile in %s (untrusted content)"
+           (buffer-name)))
   (when elisp-flymake--byte-compile-process
     (when (process-live-p elisp-flymake--byte-compile-process)
       (kill-process elisp-flymake--byte-compile-process)))