GH-115322: Add missing audit hooks (GH-115624)

Add extra audit hooks to catch C function calling from ctypes,
reading/writing files through readline and executing external
programs through _posixsubprocess.

* Make audit-tests for open pass when readline.append_history_file is unavailable
* Less direct testing of _posixsubprocess for audit hooks
* Also remove the audit hook from call_cdeclfunction now that _ctypes_callproc does it instead.
* reword the NEWS entry.
* mention readline in NEWS
* add versionchanged markers
* fix audit_events.rst versionadded
* doc lint

---------

Co-authored-by: Gregory P. Smith <greg@krypto.org>

Doc/library/audit_events.rst

@@ -23,25 +23,30 @@ information on handling these events.
 The following events are raised internally and do not correspond to any
 public API of CPython:
 
-+--------------------------+-------------------------------------------+
-| Audit event              | Arguments                                 |
-+==========================+===========================================+
-| _winapi.CreateFile       | ``file_name``, ``desired_access``,        |
-|                          | ``share_mode``, ``creation_disposition``, |
-|                          | ``flags_and_attributes``                  |
-+--------------------------+-------------------------------------------+
-| _winapi.CreateJunction   | ``src_path``, ``dst_path``                |
-+--------------------------+-------------------------------------------+
-| _winapi.CreateNamedPipe  | ``name``, ``open_mode``, ``pipe_mode``    |
-+--------------------------+-------------------------------------------+
-| _winapi.CreatePipe       |                                           |
-+--------------------------+-------------------------------------------+
-| _winapi.CreateProcess    | ``application_name``, ``command_line``,   |
-|                          | ``current_directory``                     |
-+--------------------------+-------------------------------------------+
-| _winapi.OpenProcess      | ``process_id``, ``desired_access``        |
-+--------------------------+-------------------------------------------+
-| _winapi.TerminateProcess | ``handle``, ``exit_code``                 |
-+--------------------------+-------------------------------------------+
-| ctypes.PyObj_FromPtr     | ``obj``                                   |
-+--------------------------+-------------------------------------------+
++----------------------------+-------------------------------------------+
+| Audit event                | Arguments                                 |
++============================+===========================================+
+| _winapi.CreateFile         | ``file_name``, ``desired_access``,        |
+|                            | ``share_mode``, ``creation_disposition``, |
+|                            | ``flags_and_attributes``                  |
++----------------------------+-------------------------------------------+
+| _winapi.CreateJunction     | ``src_path``, ``dst_path``                |
++----------------------------+-------------------------------------------+
+| _winapi.CreateNamedPipe    | ``name``, ``open_mode``, ``pipe_mode``    |
++----------------------------+-------------------------------------------+
+| _winapi.CreatePipe         |                                           |
++----------------------------+-------------------------------------------+
+| _winapi.CreateProcess      | ``application_name``, ``command_line``,   |
+|                            | ``current_directory``                     |
++----------------------------+-------------------------------------------+
+| _winapi.OpenProcess        | ``process_id``, ``desired_access``        |
++----------------------------+-------------------------------------------+
+| _winapi.TerminateProcess   | ``handle``, ``exit_code``                 |
++----------------------------+-------------------------------------------+
+| _posixsubprocess.fork_exec | ``exec_list``, ``args``, ``env``          |
++----------------------------+-------------------------------------------+
+| ctypes.PyObj_FromPtr       | ``obj``                                   |
++----------------------------+-------------------------------------------+
+
+.. versionadded:: next
+   The ``_posixsubprocess.fork_exec`` internal audit event.

Doc/library/ctypes.rst

@@ -1779,7 +1779,8 @@ in :mod:`!ctypes`) which inherits from the private :class:`_CFuncPtr` class:
 
 .. audit-event:: ctypes.call_function func_pointer,arguments foreign-functions
 
-   Some ways to invoke foreign function calls may raise an auditing event
+   Some ways to invoke foreign function calls as well as some of the
+   functions in this module may raise an auditing event
    ``ctypes.call_function`` with arguments ``function pointer`` and ``arguments``.
 
 .. _ctypes-function-prototypes:

Doc/library/readline.rst

@@ -72,6 +72,12 @@ The following functions relate to the init file and user configuration:
 
    Execute a readline initialization file. The default filename is the last filename
    used. This calls :c:func:`!rl_read_init_file` in the underlying library.
+   It raises an :ref:`auditing event <auditing>` ``open`` with the file name
+   if given, and :code:`"<readline_init_file>"` otherwise, regardless of
+   which file the library resolves.
+
+   .. versionchanged:: next
+      The auditing event was added.
 
 
 Line buffer
@@ -109,14 +115,24 @@ The following functions operate on a history file:
 
    Load a readline history file, and append it to the history list.
    The default filename is :file:`~/.history`.  This calls
-   :c:func:`!read_history` in the underlying library.
+   :c:func:`!read_history` in the underlying library
+   and raises an :ref:`auditing event <auditing>` ``open`` with the file
+   name if given and :code:`"~/.history"` otherwise.
+
+   .. versionchanged:: next
+      The auditing event was added.
 
 
 .. function:: write_history_file([filename])
 
    Save the history list to a readline history file, overwriting any
    existing file.  The default filename is :file:`~/.history`.  This calls
-   :c:func:`!write_history` in the underlying library.
+   :c:func:`!write_history` in the underlying library and raises an
+   :ref:`auditing event <auditing>` ``open`` with the file name if given and
+   :code:`"~/.history"` otherwise.
+
+   .. versionchanged:: next
+      The auditing event was added.
 
 
 .. function:: append_history_file(nelements[, filename])
@@ -125,10 +141,14 @@ The following functions operate on a history file:
    :file:`~/.history`.  The file must already exist.  This calls
    :c:func:`!append_history` in the underlying library.  This function
    only exists if Python was compiled for a version of the library
-   that supports it.
+   that supports it. It raises an :ref:`auditing event <auditing>` ``open``
+   with the file name if given and :code:`"~/.history"` otherwise.
 
    .. versionadded:: 3.5
 
+   .. versionchanged:: next
+      The auditing event was added.
+
 
 .. function:: get_history_length()
               set_history_length(length)

Lib/test/audit-tests.py

@@ -195,6 +195,17 @@ def test_open(testfn):
     except ImportError:
         load_dh_params = None
 
+    try:
+        import readline
+    except ImportError:
+        readline = None
+
+    def rl(name):
+        if readline:
+            return getattr(readline, name, None)
+        else:
+            return None
+
     # Try a range of "open" functions.
     # All of them should fail
     with TestHook(raise_on_events={"open"}) as hook:
@@ -204,6 +215,14 @@ def test_open(testfn):
             (open, 3, "wb"),
             (open, testfn, "w", -1, None, None, None, False, lambda *a: 1),
             (load_dh_params, testfn),
+            (rl("read_history_file"), testfn),
+            (rl("read_history_file"), None),
+            (rl("write_history_file"), testfn),
+            (rl("write_history_file"), None),
+            (rl("append_history_file"), 0, testfn),
+            (rl("append_history_file"), 0, None),
+            (rl("read_init_file"), testfn),
+            (rl("read_init_file"), None),
         ]:
             if not fn:
                 continue
@@ -229,6 +248,14 @@ def test_open(testfn):
                 (3, "w"),
                 (testfn, "w"),
                 (testfn, "rb") if load_dh_params else None,
+                (testfn, "r") if readline else None,
+                ("~/.history", "r") if readline else None,
+                (testfn, "w") if readline else None,
+                ("~/.history", "w") if readline else None,
+                (testfn, "a") if rl("append_history_file") else None,
+                ("~/.history", "a") if rl("append_history_file") else None,
+                (testfn, "r") if readline else None,
+                ("<readline_init_file>", "r") if readline else None,
             ]
             if i is not None
         ],
@@ -278,6 +305,37 @@ def test_mmap():
         assertEqual(hook.seen[0][1][:2], (-1, 8))
 
 
+def test_ctypes_call_function():
+    import ctypes
+    import _ctypes
+
+    with TestHook() as hook:
+        _ctypes.call_function(ctypes._memmove_addr, (0, 0, 0))
+        assert ("ctypes.call_function", (ctypes._memmove_addr, (0, 0, 0))) in hook.seen
+
+        ctypes.CFUNCTYPE(ctypes.c_voidp)(ctypes._memset_addr)(1, 0, 0)
+        assert ("ctypes.call_function", (ctypes._memset_addr, (1, 0, 0))) in hook.seen
+
+    with TestHook() as hook:
+        ctypes.cast(ctypes.c_voidp(0), ctypes.POINTER(ctypes.c_char))
+        assert "ctypes.call_function" in hook.seen_events
+
+    with TestHook() as hook:
+        ctypes.string_at(id("ctypes.string_at") + 40)
+        assert "ctypes.call_function" in hook.seen_events
+        assert "ctypes.string_at" in hook.seen_events
+
+
+def test_posixsubprocess():
+    import multiprocessing.util
+
+    exe = b"xxx"
+    args = [b"yyy", b"zzz"]
+    with TestHook() as hook:
+        multiprocessing.util.spawnv_passfds(exe, args, ())
+        assert ("_posixsubprocess.fork_exec", ([exe], args, None)) in hook.seen
+
+
 def test_excepthook():
     def excepthook(exc_type, exc_value, exc_tb):
         if exc_type is not RuntimeError:

Lib/test/test_audit.py

@@ -80,6 +80,14 @@ class AuditTest(unittest.TestCase):
     def test_mmap(self):
         self.do_test("test_mmap")
 
+    def test_ctypes_call_function(self):
+        import_helper.import_module("ctypes")
+        self.do_test("test_ctypes_call_function")
+
+    def test_posixsubprocess(self):
+        import_helper.import_module("_posixsubprocess")
+        self.do_test("test_posixsubprocess")
+
     def test_excepthook(self):
         returncode, events, stderr = self.run_python("test_excepthook")
         if not returncode:

Misc/NEWS.d/next/Security/2024-02-18-02-53-25.gh-issue-115322.Um2Sjx.rst

@@ -0,0 +1,4 @@
+The underlying extension modules behind :mod:`readline`:, :mod:`subprocess`,
+and :mod:`ctypes` now raise audit events on previously uncovered code paths
+that could lead to file system access related to C function calling and
+external binary execution.

Modules/_ctypes/callproc.c

@@ -1198,6 +1198,12 @@ PyObject *_ctypes_callproc(ctypes_state *st,
     void **avalues;
     PyObject *retval = NULL;
 
+    // Both call_function and call_cdeclfunction call us:
+    if (PySys_Audit("ctypes.call_function", "nO",
+                    (Py_ssize_t)pProc, argtuple) < 0) {
+        return NULL;
+    }
+
     n = argcount = PyTuple_GET_SIZE(argtuple);
 #ifdef MS_WIN32
     /* an optional COM object this pointer */
@@ -1673,10 +1679,6 @@ call_function(PyObject *self, PyObject *args)
                           &_parse_voidp, &func,
                           &PyTuple_Type, &arguments))
         return NULL;
-    if (PySys_Audit("ctypes.call_function", "nO",
-                    (Py_ssize_t)func, arguments) < 0) {
-        return NULL;
-    }
 
     ctypes_state *st = get_module_state(self);
     result = _ctypes_callproc(st,
@@ -1710,10 +1712,6 @@ call_cdeclfunction(PyObject *self, PyObject *args)
                           &_parse_voidp, &func,
                           &PyTuple_Type, &arguments))
         return NULL;
-    if (PySys_Audit("ctypes.call_function", "nO",
-                    (Py_ssize_t)func, arguments) < 0) {
-        return NULL;
-    }
 
     ctypes_state *st = get_module_state(self);
     result = _ctypes_callproc(st,

Modules/_posixsubprocess.c

@@ -1203,6 +1203,19 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
         goto cleanup;
     }
 
+    /* NOTE: user-defined classes may be able to present different
+     * executable/argument/env lists to the eventual exec as to this hook.
+     * The audit hook receives the original object, so would nevertheless
+     * be able to detect weird behaviour, hence we do not add extra
+     * complexity or performance penalties to attempt to avoid this. */
+    if (PySys_Audit("_posixsubprocess.fork_exec",
+                    "OOO",
+                    executable_list,
+                    process_args,
+                    env_list) < 0) {
+        goto cleanup;
+    }
+
     /* This must be the last thing done before fork() because we do not
      * want to call PyOS_BeforeFork() if there is any chance of another
      * error leading to the cleanup: code without calling fork(). */

Modules/readline.c

@@ -254,10 +254,21 @@ readline_read_init_file_impl(PyObject *module, PyObject *filename_obj)
     if (filename_obj != Py_None) {
         if (!PyUnicode_FSConverter(filename_obj, &filename_bytes))
             return NULL;
+        if (PySys_Audit("open", "OCi", filename_obj, 'r', 0) < 0) {
+            return NULL;
+        }
         errno = rl_read_init_file(PyBytes_AS_STRING(filename_bytes));
         Py_DECREF(filename_bytes);
-    } else
+    } else {
+        /* We have the choice to either try to exactly reproduce the
+         * logic to find the filename, ignore it, or provide a dummy value.
+         * In contract to the history file manipulations, there's no
+         * clear default to choose. */
+        if (PySys_Audit("open", "sCi", "<readline_init_file>", 'r', 0) < 0) {
+            return NULL;
+        }
         errno = rl_read_init_file(NULL);
+    }
     if (errno)
         return PyErr_SetFromErrno(PyExc_OSError);
     disable_bracketed_paste();
@@ -286,10 +297,19 @@ readline_read_history_file_impl(PyObject *module, PyObject *filename_obj)
     if (filename_obj != Py_None) {
         if (!PyUnicode_FSConverter(filename_obj, &filename_bytes))
             return NULL;
+        if (PySys_Audit("open", "OCi", filename_obj, 'r', 0) < 0) {
+            return NULL;
+        }
         errno = read_history(PyBytes_AS_STRING(filename_bytes));
         Py_DECREF(filename_bytes);
-    } else
+    } else {
+        /* Use the documented default filename here,
+         * even though readline expands it different internally. */
+        if (PySys_Audit("open", "sCi", "~/.history", 'r', 0) < 0) {
+            return NULL;
+        }
         errno = read_history(NULL);
+    }
     if (errno)
         return PyErr_SetFromErrno(PyExc_OSError);
     Py_RETURN_NONE;
@@ -322,9 +342,17 @@ readline_write_history_file_impl(PyObject *module, PyObject *filename_obj)
         if (!PyUnicode_FSConverter(filename_obj, &filename_bytes))
             return NULL;
         filename = PyBytes_AS_STRING(filename_bytes);
+        if (PySys_Audit("open", "OCi", filename_obj, 'w', 0) < 0) {
+            return NULL;
+        }
     } else {
         filename_bytes = NULL;
         filename = NULL;
+        /* Use the documented default filename here,
+         * even though readline expands it different internally. */
+        if (PySys_Audit("open", "sCi", "~/.history", 'w', 0) < 0) {
+            return NULL;
+        }
     }
     errno = err = write_history(filename);
     int history_length = FT_ATOMIC_LOAD_INT_RELAXED(_history_length);
@@ -371,9 +399,17 @@ readline_append_history_file_impl(PyObject *module, int nelements,
         if (!PyUnicode_FSConverter(filename_obj, &filename_bytes))
             return NULL;
         filename = PyBytes_AS_STRING(filename_bytes);
+        if (PySys_Audit("open", "OCi", filename_obj, 'a', 0) < 0) {
+            return NULL;
+        }
     } else {
         filename_bytes = NULL;
         filename = NULL;
+        /* Use the documented default filename here,
+         * even though readline expands it different internally. */
+        if (PySys_Audit("open", "sCi", "~/.history", 'a', 0) < 0) {
+            return NULL;
+        }
     }
     errno = err = append_history(
         nelements - libedit_append_replace_history_offset, filename);