It is currently possible to discover the internal list of audit hooks using gc module functions, like gc.get_objects(), and so remove an audit hooks, whereas it is supposed to not be possible. The PEP 578 states: "Hooks cannot be removed or replaced."

Rather than attempting to fix this specific vulnerability, I suggest to add new audit events on the following gc functions:

* gc.get_objects()
* gc.get_referrers()
* gc.get_referents()

These functions are "dangerous" since they can expose Python objects in an inconsistent state. In the past, we add multiple bugs related to "internal" tuples which were not fully initialized (but already tracked by the GC). See bpo-15108 for an example.

Note: if someone wants to address the ability to remove an audit hook, the internal list can be modified to not be a Python object.

> Rather than attempting to fix this specific vulnerability, I suggest to add new audit events on the following gc functions:

Makes sense, I will prepare a PR today

> Note: if someone wants to address the ability to remove an audit hook, the internal list can be modified to not be a Python object.

I wouldn't bother. There are other ways to modify data structures, e.g. poke into process memory.

Thanks, Pablo! Nice patch.

This can be backported, btw. New audit events are allowed to be added in minor releases (they just cannot be changed).

New changeset b4f9089d4aa787c5b74134c98e5f0f11d9e63095 by Pablo Galindo in branch 'master':
bpo-43439: Add audit hooks for gc functions (GH-24794)
https://github.com/python/cpython/commit/b4f9089d4aa787c5b74134c98e5f0f11d9e63095

New changeset a6d0182879d0bf275c4feb38b57f73236ab9c06c by Pablo Galindo in branch '3.8':
[3.8] bpo-43439: Add audit hooks for gc functions (GH-24794). (GH-24810)
https://github.com/python/cpython/commit/a6d0182879d0bf275c4feb38b57f73236ab9c06c

New changeset f814675376318e0bf9e14fc62826a113cb4ca652 by Pablo Galindo in branch '3.9':
[3.9] bpo-43439: Add audit hooks for gc functions (GH-24794). (GH-24811)
https://github.com/python/cpython/commit/f814675376318e0bf9e14fc62826a113cb4ca652

Thanks, Pablo and Victor!

There is a minor issue here. For gc.get_referrers and gc.get_referents, probably the format code for PySys_Audit should be "(O)" instead of "O". Typically the tuple `args` passed to the hook functions are fixed-length as described in the audit events table. Here `objs` itself is a tuple (containing variable-length arguments) and directly passed to the audit hook with being wrapped by another layer of tuple. If the hook function is to receive a variable-length tuple, probably the documentation should be updated to mention this.

Steve, do you think that makes sense? If so, I will create a batch of PRs to correct it.

In addition to the consistency with existing audit hook signatures, there may also be another benefit of wrapping it with a tuple of length 1. If gc.get_referrers or gc.get_referents happens to gain a new keyword-only argument in the future, we may need to add the new argument to the hook args. Extending the `(objs,)` tuple to `(objs, new_kwarg)` is a bit more elegant than appending the `new_kwarg` to the end of the `objs` tuple itself (considering a hook function which tries to be compatible with both the old and the new signature).

Passing a tuple as "O" just means it gets passed as-is, without wrapping it again. And yeah, I thought that was right here, but it's not. I didn't realise it's a varargs argument. So yeah, it should be wrapped again so that only a single argument is being passed to the hooks.

Alternatively, raising one event for each object would also be valid. These functions behave identically for "f(a, b)" and "f(a) + f(b)", so raising the event for each object before traversing it would simplify hooks (but have more of a performance impact... which I suspect is totally irrelevant here anyway, so I'd be +1.1 on this approach vs. +1 on passing the args as a single argument).

If new arguments are added later, we need to add new events. Defining event schemas as "arbitrary arguments that may change is the future" is totally against the intended design.

I think I prefer to raise a single event, because it match more closely the actual call, is a bit faster and more straighfoward. I will create a PR soon

New changeset 9c376bc1c4c8bcddb0bc4196b79ec8c75da494a8 by Pablo Galindo in branch 'master':
bpo-43439: Wrapt the tuple in the audit events for the gc module (GH-24836)
https://github.com/python/cpython/commit/9c376bc1c4c8bcddb0bc4196b79ec8c75da494a8

New changeset 1e7a47ab86d5d6a5103e67ba71389f6daa18ea2d by Pablo Galindo in branch '3.8':
[3.8] bpo-43439: Wrapt the tuple in the audit events for the gc module (GH-24836) (GH24854)
https://github.com/python/cpython/commit/1e7a47ab86d5d6a5103e67ba71389f6daa18ea2d

New changeset e6bf1e1001a6844a36f2f90f58ab12b9e09e3887 by Pablo Galindo in branch '3.9':
[3.9] bpo-43439: Wrapt the tuple in the audit events for the gc module (GH-24836) (GH-24855)
https://github.com/python/cpython/commit/e6bf1e1001a6844a36f2f90f58ab12b9e09e3887

stage: resolved -> needs patch

stage: patch review -> resolved