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`.

(cherry picked from commit b5158bd191422e46273c4d9412f2bf097e2da2e0)

diff --git a/lisp/files.el b/lisp/files.el
index aff4a36c716be9d4d990ffba5b488dac2c615073..f9889762148d5729a6d90d839cd48493efa6831b 100644 (file)
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -712,6 +712,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 9128595c4ebf8a3a77dc1c86fab584761422de14..7674836a1a255da12ab8c8770cc01b03f59d5321 100644 (file)
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -558,6 +558,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
@@ -573,22 +601,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 nil
-                    (apply expander form args)
-                  (error 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)
@@ -2156,6 +2170,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)))