master c3a2080 2/2: Trim and explain set of safe forms for 'unsafep' (bug#44018)

[Mattias Engdegård]

branch: master
commit c3a20804a81826ec091a4a096c1987a61e412580
Author: Mattias Engdegård <mattiase@acm.org>
Commit: Mattias Engdegård <mattiase@acm.org>

    Trim and explain set of safe forms for 'unsafep' (bug#44018)
    
    * lisp/emacs-lisp/unsafep.el:
    Add comment explaining the policy for which forms can be considered
    'safe' in the sense of unsafep.  Remove ones that didn't make the cut:
    
     play-sound-file (large attack surface)
     catch, throw (alter program flow, inject data)
     replace-regexp-in-string (execute arbitary code)
     error, signal (deceptive messages)
    
    * test/lisp/emacs-lisp/unsafep-tests.el (unsafep-tests--unsafe):
    Add test cases.
    * etc/NEWS: Announce the change.
---
 etc/NEWS                              |  5 +++++
 lisp/emacs-lisp/unsafep.el            | 32 ++++++++++++++++++++++++++++----
 test/lisp/emacs-lisp/unsafep-tests.el | 12 ++++++++++++
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 4cc66ae..4435d05 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1835,6 +1835,11 @@ file can affect code in another.  For details, see the 
manual section
 ---
 ** 'unload-feature' now also tries to undo additions to buffer-local hooks.
 
+---
+** Some functions are no longer considered safe by 'unsafep':
+'replace-regexp-in-string', 'catch', 'throw', 'error', 'signal'
+and 'play-sound-file'.
+
 
 * Changes in Emacs 28.1 on Non-Free Operating Systems
 
diff --git a/lisp/emacs-lisp/unsafep.el b/lisp/emacs-lisp/unsafep.el
index e707714..c4db86a 100644
--- a/lisp/emacs-lisp/unsafep.el
+++ b/lisp/emacs-lisp/unsafep.el
@@ -91,17 +91,41 @@
 in the parse.")
 (put 'unsafep-vars 'risky-local-variable t)
 
-;;Other safe functions
+;; Other safe forms.
+;;
+;; A function, macro or special form may be put here only if all of
+;; the following statements are true:
+;;
+;; * It is not already marked `pure' or `side-effect-free', or handled
+;;   explicitly by `unsafep'.
+;;
+;; * It is not inherently unsafe; eg, would allow the execution of
+;;   arbitrary code, interact with the file system, network or other
+;;   processes, or otherwise exfiltrate information from the running
+;;   Emacs process or manipulate the user's environment.
+;;
+;; * It does not have side-effects that can make other code behave in
+;;   unsafe and/or unexpected ways; eg, set variables, mutate data, or
+;;   change control flow.
+;;   Any side effect must be innocuous; altering the match data is
+;;   explicitly permitted.
+;;
+;; * It does not allow Emacs to behave deceptively to the user; eg,
+;;   display arbitrary messages.
+;;
+;; * It does not present a potentially large attack surface; eg,
+;;   play arbitrary audio files.
+
 (dolist (x '(;;Special forms
-            and catch if or prog1 prog2 progn while unwind-protect
+            and if or prog1 prog2 progn while unwind-protect
             ;;Safe subrs that have some side-effects
-            ding error random signal sleep-for string-match throw
+            ding random sleep-for string-match
             ;;Defsubst functions from subr.el
             caar cadr cdar cddr
             ;;Macros from subr.el
             save-match-data unless when
             ;;Functions from subr.el that have side effects
-            split-string replace-regexp-in-string play-sound-file))
+            split-string))
   (put x 'safe-function t))
 
 ;;;###autoload
diff --git a/test/lisp/emacs-lisp/unsafep-tests.el 
b/test/lisp/emacs-lisp/unsafep-tests.el
index dde0e02..06c40d2 100644
--- a/test/lisp/emacs-lisp/unsafep-tests.el
+++ b/test/lisp/emacs-lisp/unsafep-tests.el
@@ -105,6 +105,18 @@
       . (variable (x)))
     ( (let (1) 2)
       . (variable 1))
+    ( (error "asdf")
+      . #'error)
+    ( (signal 'error "asdf")
+      . #'signal)
+    ( (throw 'asdf)
+      . #'throw)
+    ( (catch 'asdf 17)
+      . #'catch)
+    ( (play-sound-file "asdf")
+      . #'play-sound-file)
+    ( (replace-regexp-in-string "a" "b")
+      . #'replace-regexp-in-string)
     )
   "A-list of (FORM . REASON)... that `unsafep' should decide are unsafe.")