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

'y' does not check for embedded NUL bytes #52838

Closed
pitrou opened this issue May 1, 2010 · 6 comments
Closed

'y' does not check for embedded NUL bytes #52838

pitrou opened this issue May 1, 2010 · 6 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@pitrou
Copy link
Member

pitrou commented May 1, 2010

BPO 8592
Nosy @loewis, @pitrou, @vstinner
Files
  • y_format.patch
  • 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/loewis'
    closed_at = <Date 2010-06-13.20:06:19.510>
    created_at = <Date 2010-05-01.18:04:33.327>
    labels = ['interpreter-core', 'type-bug']
    title = "'y' does not check for embedded NUL bytes"
    updated_at = <Date 2010-06-13.20:06:19.509>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2010-06-13.20:06:19.509>
    actor = 'vstinner'
    assignee = 'loewis'
    closed = True
    closed_date = <Date 2010-06-13.20:06:19.510>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2010-05-01.18:04:33.327>
    creator = 'pitrou'
    dependencies = []
    files = ['17636']
    hgrepos = []
    issue_num = 8592
    keywords = ['patch']
    message_count = 6.0
    messages = ['104734', '104991', '104992', '107368', '107616', '107747']
    nosy_count = 3.0
    nosy_names = ['loewis', 'pitrou', 'vstinner']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'needs patch'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue8592'
    versions = ['Python 3.1', 'Python 3.2']

    @pitrou
    Copy link
    Member Author

    pitrou commented May 1, 2010

    The documentation for the 'y' format (PyArg_ParseTuple and friends) states that:

    « The bytes object must not contain embedded NUL bytes; if it does, a TypeError exception is raised. »

    But, reading Python/getargs.c, the strlen() check is actually missing in the code for 'y'.

    @pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels May 1, 2010
    @vstinner
    Copy link
    Member

    vstinner commented May 5, 2010

    Same issue for y#:

    y# (...) This variant on s# doesn’t accept Unicode objects, only bytes-like objects.

    s# (...) The string may contain embedded null bytes.

    --

    y* might mention that it accepts embedded null bytes.

    --

    grep 'PyArg_Parse[^"]\+"[^:;)"]y[^*]' */.c finds only usage of y# (no usage of y format):

    • mmap_gfind(), mmap_write_method()
    • oss_write(), oss_writeall()
    • in getsockaddrarg() with s->sock_family==AF_PACKET
    • in sock_setsockopt() if the option name is a string
    • socket_inet_ntoa(), socket_inet_ntop()

    These functions have to support embedded null bytes. So I think that y# should specify explicitly that embedded null bytes are accepted.

    @vstinner
    Copy link
    Member

    vstinner commented May 5, 2010

    See also bpo-8215.

    @vstinner
    Copy link
    Member

    vstinner commented Jun 9, 2010

    See also bpo-8850: Remove "w" format of PyParse_ParseTuple().

    --

    About "y": the parser HAVE TO check for embedded NUL bytes, because the caller doesn't know the size of the buffer (and strlen() would give the wrong size).

    @vstinner
    Copy link
    Member

    Attached patch fixes the initial problem: raise an error if the byte strings embeds a NUL-byte.

    @vstinner
    Copy link
    Member

    I commited a bigger patch: r81973 not only fixes "y" format, but also "u" and "Z". It does also add a lot of tests in test_getargs2.py for many string formats (not all, eg. "es" is not tested).

    Even if I consider this as a bugfix, I don't want to backport to 3.1 because it might break programs which rely on this strange behaviour.

    @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
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants