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`.
This commit is contained in:
Stefan Monnier 2024-12-10 16:26:31 -05:00
parent 6942599dbb
commit b5158bd191
3 changed files with 101 additions and 17 deletions

View file

@ -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,

View file

@ -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)

View file

@ -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)))