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

PyArg_ParseTuple() false-returns SUCCESS though SystemError and missing data (when PY_SSIZE_T_CLEAN not #define'd) #87487

Closed
kxrob mannequin opened this issue Feb 25, 2021 · 5 comments
Labels
3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@kxrob
Copy link
Mannequin

kxrob mannequin commented Feb 25, 2021

BPO 43321
Nosy @vstinner, @methane, @kxrob
PRs
  • bpo-43321: Fix SystemError in getargs.c #24656
  • 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 = None
    closed_at = <Date 2021-02-27.11:31:19.779>
    created_at = <Date 2021-02-25.16:05:25.050>
    labels = ['interpreter-core', '3.10', 'type-crash']
    title = "PyArg_ParseTuple() false-returns SUCCESS though SystemError and missing data (when PY_SSIZE_T_CLEAN  not #define'd)"
    updated_at = <Date 2021-03-08.22:02:34.293>
    user = 'https://github.com/kxrob'

    bugs.python.org fields:

    activity = <Date 2021-03-08.22:02:34.293>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-02-27.11:31:19.779>
    closer = 'methane'
    components = ['Interpreter Core']
    creation = <Date 2021-02-25.16:05:25.050>
    creator = 'kxrob'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43321
    keywords = ['patch']
    message_count = 5.0
    messages = ['387678', '387696', '387706', '387776', '388312']
    nosy_count = 3.0
    nosy_names = ['vstinner', 'methane', 'kxrob']
    pr_nums = ['24656']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue43321'
    versions = ['Python 3.10']

    @kxrob
    Copy link
    Mannequin Author

    kxrob mannequin commented Feb 25, 2021

    When PY_SSIZE_T_CLEAN is not #defined in Py3.10, PyArg_ParseTuple() etc. sets a SystemError but the return value says 1 (=SUCCESS)!
    => Causes terrific crashes with unfilled variables - instead of a clean Python exception.

    Background: pywin32 suffers in masses from the drop of int support for the s#/y#/et#... formats in PyArg_ParseTuple() etc. (PY_SSIZE_T_CLEAN is required in Py3.10). And only part of the source is already PY_SSIZE_T_CLEAN. Now it is very difficult to even run tests and weed out / check, because of freezes instead of nice Python exceptions.

    => Instead of preventing such freezes, the PY_SSIZE_T_CLEAN mechanism became the opposite: a clever trap, a sword of Damocles :)

    The cause is in getargs.c:

    =================== getargs.c / convertsimple() ====

    #define REQUIRE_PY_SSIZE_T_CLEAN \
        if (!(flags & FLAG_SIZE_T)) { \
            PyErr_SetString(PyExc_SystemError, \
                            "PY_SSIZE_T_CLEAN macro must be defined for '#' formats"); \
            return NULL; \
        }
    #define RETURN_ERR_OCCURRED return msgbuf

    ===================

    => The return NULL is further processed as no msg NULL -> no error.
    => Perhaps it should be a return converterr(...) or return sstc_system_error(...) !?

    @kxrob kxrob mannequin added 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Feb 25, 2021
    @methane
    Copy link
    Member

    methane commented Feb 26, 2021

    Thank you for reporting.

    @vstinner Can we provide a nice C traceback too?

    @methane
    Copy link
    Member

    methane commented Feb 26, 2021

    I checked the warning, and now I think Python traceback is fine.

    PyArg_Parse*() is used on top of the C function. So python traceback is enough to find which function is using '#' without Py_SSIZE_T_CLEAN.

    @methane
    Copy link
    Member

    methane commented Feb 27, 2021

    New changeset c71d24f by Inada Naoki in branch 'master':
    bpo-43321: Fix SystemError in getargs.c (GH-24656)
    c71d24f

    @vstinner
    Copy link
    Member

    vstinner commented Mar 8, 2021

    Thanks for fixing my bug ;-) (bpo-40943)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants