Skip to content

Conversation

@RobinJadoul
Copy link
Contributor

@RobinJadoul RobinJadoul commented Feb 18, 2024

GH-115322: Add missing audit hooks

I've left out the potential ctypes.cdata/function event for now, as constructing a function without notice is not as bad
when you cannot call it without audit hook anymore. Calling some functions in ctypes (as far as I can see: (w)string_at) now have two associated hooks, going first through the generic ctypes.call_function and then their preexisting specific hook (ctypes.(w)string_at). It would be nice to associated a ctypes.call_function with its function name, but this isn't possible in general (as you could technically call into arbitrary addresses), and I couldn't find a nice way to do it even in the "nice" case.

For _posixsubprocess.fork_exec, I instead opted to introduce a new event of its own, as wrangling the slightly weird setup with executable_list and passing env as a list rather than the usual dict makes it more annoying to (cheaply) map onto the semantics of the existing events. If constructing an explicit dict or passing an env list to those would be prefered, it should be possible to assume the worst case for the executable_list and do a hook for each element.
In the "normal" case, going through multiprocessing.util.spawnv_passfds, only a single element would be passed either way.

readline has a few of its own problems, mostly when dealing with filename arguments being set to None. For the *_history_file functions, I opted to hardcode the ~/.history path that can also be found in the documentation, even though the logic inside readline is slightly more elaborate, e.g. finding a different path when on windows, and expanding out the home directory. For read_init_file(None), the logic is even more convoluted, and pointing out the exact path would require replicating it and keeping up with any possible future updates. Instead, I chose to represent this by <readline_init_file>.


📚 Documentation preview 📚: https://cpython-previews--115624.org.readthedocs.build/

Add extra audit hooks to catch C function calling from ctypes,
reading/writing files through readline and executing external
programs through _posixsubprocess.
@RobinJadoul RobinJadoul requested a review from gpshead as a code owner February 18, 2024 02:45
@ghost
Copy link

ghost commented Feb 18, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Feb 18, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@gpshead gpshead self-assigned this Feb 22, 2024
@RobinJadoul RobinJadoul requested a review from gpshead March 4, 2024 16:33
@gpshead gpshead added the needs backport to 3.13 bugs and security fixes label May 20, 2024
@gpshead gpshead enabled auto-merge (squash) May 20, 2024 20:10
@gpshead gpshead added type-bug An unexpected behavior, bug, or error needs backport to 3.12 only security fixes labels May 20, 2024
@tomasr8 tomasr8 removed the needs backport to 3.12 only security fixes label Apr 10, 2025
@tomasr8
Copy link
Member

tomasr8 commented Apr 13, 2025

cc @gpshead you enabled auto-merge but the docs workflow failed so it never got merged.

@gpshead gpshead removed the needs backport to 3.13 bugs and security fixes label Apr 13, 2025
@gpshead gpshead added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Apr 13, 2025
@gpshead
Copy link
Member

gpshead commented Apr 13, 2025

this PR is ready but we're having github CI issues all over right now.

@gpshead gpshead added type-bug An unexpected behavior, bug, or error and removed type-bug An unexpected behavior, bug, or error labels Apr 13, 2025
@gpshead gpshead disabled auto-merge April 13, 2025 19:52
@gpshead gpshead closed this Apr 13, 2025
@gpshead gpshead reopened this Apr 13, 2025
@gpshead gpshead enabled auto-merge (squash) April 13, 2025 20:12
@gpshead gpshead merged commit 2666a06 into python:main Apr 13, 2025
42 checks passed
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM Raspbian 3.x (tier-3) has failed when building commit 2666a06.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/424/builds/10197) and take a look at the build logs.
  4. Check if the failure is related to this commit (2666a06) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/424/builds/10197

Failed tests:

  • test_audit

Failed subtests:

  • test_ctypes_call_function - test.test_audit.AuditTest.test_ctypes_call_function

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/var/lib/buildbot/workers/3.x.gps-raspbian.nondebug/build/Lib/test/audit-tests.py", line 653, in <module>
    globals()[test](*sys.argv[2:])
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
  File "/var/lib/buildbot/workers/3.x.gps-raspbian.nondebug/build/Lib/test/audit-tests.py", line 314, in test_ctypes_call_function
    assert ("ctypes.call_function", (ctypes._memmove_addr, (0, 0, 0))) in hook.seen
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError


Traceback (most recent call last):
  File "/var/lib/buildbot/workers/3.x.gps-raspbian.nondebug/build/Lib/test/test_audit.py", line 85, in test_ctypes_call_function
    self.do_test("test_ctypes_call_function")
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/buildbot/workers/3.x.gps-raspbian.nondebug/build/Lib/test/test_audit.py", line 39, in do_test
    self.fail(stderr)
    ~~~~~~~~~^^^^^^^^
AssertionError: Traceback (most recent call last):
  File "/var/lib/buildbot/workers/3.x.gps-raspbian.nondebug/build/Lib/test/audit-tests.py", line 653, in <module>
    globals()[test](*sys.argv[2:])
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
  File "/var/lib/buildbot/workers/3.x.gps-raspbian.nondebug/build/Lib/test/audit-tests.py", line 314, in test_ctypes_call_function
    assert ("ctypes.call_function", (ctypes._memmove_addr, (0, 0, 0))) in hook.seen
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants