From cbf43ac63e75e957bda2f7a53fc89e33a9d53a25 Mon Sep 17 00:00:00 2001 From: Stefan Monnier Date: Tue, 10 Dec 2024 16:26:31 -0500 Subject: 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) --- lisp/files.el | 49 ++++++++++++++++++++++++++++++++++ lisp/progmodes/elisp-mode.el | 51 ++++++++++++++++++++++++++++-------- 2 files changed, 89 insertions(+), 11 deletions(-) diff --git a/lisp/files.el b/lisp/files.el index e127e99d410..c451a4f063f 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -631,6 +631,55 @@ untrusted-content 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 9c5c4230212..1ee1a8c37f2 100644 --- a/lisp/progmodes/elisp-mode.el +++ b/lisp/progmodes/elisp-mode.el @@ -383,6 +383,36 @@ elisp--local-variables-1 (defvar warning-minimum-log-level) +(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)))) + (mcm-advice (lambda (_handler form) form))) + (unwind-protect + ;; Silence any macro expansion errors when + ;; attempting completion at point (bug#58148). + (let ((inhibit-message t) + (warning-minimum-log-level :emergency)) + (advice-add 'macroexp--compiler-macro :override mcm-advice) + (advice-add 'macroexpand :around macroexpand-advice) + (macroexpand-all sexp)) + (advice-remove 'macroexp--compiler-macro mcm-advice) + (advice-remove 'macroexpand macroexpand-advice))))) + (defun elisp--local-variables () "Return a list of locally let-bound variables at point." (save-excursion @@ -398,17 +428,8 @@ elisp--local-variables (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 - (let ((warning-minimum-log-level :emergency)) - (advice-add 'macroexpand :around macroexpand-advice) - (macroexpand-all sexp)) - (advice-remove 'macroexpand 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) @@ -2054,6 +2075,14 @@ elisp-flymake-byte-compile "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)))