https://github.com/python/cpython/commit/2666a06d336675247e1602aeb64170b2443602ce
commit: 2666a06d336675247e1602aeb64170b2443602ce
branch: main
author: Robin Jadoul <robin.jad...@gmail.com>
committer: gpshead <g...@krypto.org>
date: 2025-04-13T21:46:20Z
summary:
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 <g...@krypto.org>
files:
A Misc/NEWS.d/next/Security/2024-02-18-02-53-25.gh-issue-115322.Um2Sjx.rst
M Doc/library/audit_events.rst
M Doc/library/ctypes.rst
M Doc/library/readline.rst
M Lib/test/audit-tests.py
M Lib/test/test_audit.py
M Modules/_ctypes/callproc.c
M Modules/_posixsubprocess.c
M Modules/readline.c
diff --git a/Doc/library/audit_events.rst b/Doc/library/audit_events.rst
index a2a90a00d0cfca..d2377a38d78c81 100644
--- a/Doc/library/audit_events.rst
+++ b/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.
diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst
index d3a81ce6bf7ea4..0bf88b0e3b66c6 100644
--- a/Doc/library/ctypes.rst
+++ b/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:
diff --git a/Doc/library/readline.rst b/Doc/library/readline.rst
index f297bdeec913a6..29e560cbc7a848 100644
--- a/Doc/library/readline.rst
+++ b/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)
diff --git a/Lib/test/audit-tests.py b/Lib/test/audit-tests.py
index af18ac2bb29c62..3d81f27e5cb46d 100644
--- a/Lib/test/audit-tests.py
+++ b/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:
diff --git a/Lib/test/test_audit.py b/Lib/test/test_audit.py
index b49938cb5008f3..2b24b5d79275fa 100644
--- a/Lib/test/test_audit.py
+++ b/Lib/test/test_audit.py
@@ -80,6 +80,14 @@ def test_cantrace(self):
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:
diff --git
a/Misc/NEWS.d/next/Security/2024-02-18-02-53-25.gh-issue-115322.Um2Sjx.rst
b/Misc/NEWS.d/next/Security/2024-02-18-02-53-25.gh-issue-115322.Um2Sjx.rst
new file mode 100644
index 00000000000000..a09e1f1fcdcab7
--- /dev/null
+++ b/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.
diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c
index 4e69c4e0a99ca2..f5db49ff4bc61c 100644
--- a/Modules/_ctypes/callproc.c
+++ b/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,
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index 1085e36cfe09b2..b542f86b6fe8da 100644
--- a/Modules/_posixsubprocess.c
+++ b/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(). */
diff --git a/Modules/readline.c b/Modules/readline.c
index dd75f256d92142..0dd99dc66c08e9 100644
--- a/Modules/readline.c
+++ b/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);
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com
