Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

_ctypes.dlsym (py_dl_sym) does not trigger audit hooks #82803

Closed
TobiasHoll mannequin opened this issue Oct 28, 2019 · 9 comments
Closed

_ctypes.dlsym (py_dl_sym) does not trigger audit hooks #82803

TobiasHoll mannequin opened this issue Oct 28, 2019 · 9 comments
Assignees
@zooba
Labels
3.8 only security fixes 3.9 only security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@TobiasHoll
Copy link
Mannequin

TobiasHoll mannequin commented Oct 28, 2019

BPO 38622
Nosy @zooba, @miss-islington, @TobiasHoll
PRs
  • bpo-38622: Add missing audit events for ctypes module #17158
  • [3.8] bpo-38622: Add missing audit events for ctypes module (GH-17158) #17242
  • bpo-38622: Ensure ctypes audit events pass tuples as a single argument #17243
  • [3.8] bpo-38622: Ensure ctypes.PyObj_FromPtr audit event passes tuples as a single argument (GH-17243) #17245
    		•
    Files
  • audit.patch: Minimal patch for this issue
  • audit-test.py: Test case
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/zooba'
closed_at = <Date 2019-11-18.22:03:04.534>
created_at = <Date 2019-10-28.18:18:57.426>
labels = ['extension-modules', 'type-bug', '3.8', '3.9']
title = '_ctypes.dlsym (py_dl_sym) does not trigger audit hooks'
updated_at = <Date 2019-11-18.22:03:04.533>
user = 'https://github.com/TobiasHoll'

    bugs.python.org fields:

    activity = <Date 2019-11-18.22:03:04.533>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2019-11-18.22:03:04.534>
closer = 'steve.dower'
components = ['Extension Modules']
creation = <Date 2019-10-28.18:18:57.426>
creator = 'tholl'
dependencies = []
files = ['48684', '48689']
hgrepos = []
issue_num = 38622
keywords = ['patch']
message_count = 9.0
messages = ['355583', '355776', '356621', '356623', '356905', '356906', '356907', '356912', '356917']
nosy_count = 3.0
nosy_names = ['steve.dower', 'miss-islington', 'tholl']
pr_nums = ['17158', '17242', '17243', '17245']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue38622'
versions = ['Python 3.8', 'Python 3.9']
    @TobiasHoll
    Copy link
    Mannequin Author

    TobiasHoll mannequin commented Oct 28, 2019

    The dlsym operation generally (e.g. when done through a ctypes.CDLL object) triggers the "ctypes.dlsym" audit event. However, using _ctypes.dlsym directly does not trigger this event. This appears to be an oversight, given that _ctypes.dlopen *does* trigger the "ctypes.dlopen" audit event.

    A (very minimal) patch is attached.

    I was not entirely sure what format the DLL handle should take when it is passed to the audit function, so for now it just turns it back into a number via PyLong_FromVoidPtr (i.e. into the same format in which it is passed into _ctypes.dlsym in the first place).

    @TobiasHoll TobiasHoll mannequin added 3.8 only security fixes 3.9 only security fixes extension-modules C modules in the Modules dir labels Oct 28, 2019
    @TobiasHoll
    Copy link
    Mannequin Author

    TobiasHoll mannequin commented Oct 31, 2019

    I probably should have done this earlier, but here's a small test case that reproduces the issue. The assert fails in vanilla Python 3.8, but passes with the patch.

    @TobiasHoll TobiasHoll mannequin added the type-bug An unexpected behavior, bug, or error label Oct 31, 2019
    @zooba
    Copy link
    Member

    zooba commented Nov 14, 2019

    Thanks! Would you like to create a PR on GitHub for this? Or are you happy for me to do it.

    @zooba
    Copy link
    Member

    zooba commented Nov 14, 2019

    Actually, it looks like we need to add events for many of the _ctypes functions, so I'll go through and do them.

    @zooba zooba self-assigned this Nov 14, 2019
    @zooba
    Copy link
    Member

    zooba commented Nov 18, 2019

    New changeset 00923c6 by Steve Dower in branch 'master':
    bpo-38622: Add missing audit events for ctypes module (GH-17158)
    00923c6

    @zooba
    Copy link
    Member

    zooba commented Nov 18, 2019

    Typically, as soon as I merge, I spot an edge case issue.

    PySys_Audit(n, "O", a) is deliberately going to treat 'a' as the tuple of arguments (when it is a tuple). This lets us simplify/optimise events where the event arguments match the function arguments exactly. If 'a' is not a tuple, it gets wrapped in one.

    When 'a' is meant to be a single argument that _might_ be a tuple, such as in PyObj_FromPtr, the format string needs to be "(O)" to ensure it is treated as a one element tuple. This is just how Py_BuildValue works - multiple elements become a tuple and the parens are optional unless you want a one-element tuple.

    @miss-islington
    Copy link
    Contributor

    New changeset 47db743 by Miss Islington (bot) in branch '3.8':
    bpo-38622: Add missing audit events for ctypes module (GH-17158)
    47db743

    @zooba
    Copy link
    Member

    zooba commented Nov 18, 2019

    New changeset dcf1f83 by Steve Dower in branch 'master':
    bpo-38622: Ensure ctypes.PyObj_FromPtr audit event passes tuples as a single argument (GH-17243)
    dcf1f83

    @miss-islington
    Copy link
    Contributor

    New changeset bec7015 by Miss Islington (bot) in branch '3.8':
    bpo-38622: Ensure ctypes.PyObj_FromPtr audit event passes tuples as a single argument (GH-17243)
    bec7015

    @zooba zooba closed this as completed Nov 18, 2019
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @zooba zooba

    Labels
    3.8 only security fixes 3.9 only security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @zooba @miss-islington