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.

Sign up for GitHub

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

socket.getaddrinfo(host) doesn't ensure that host.encode() returns a byte string #68872

Closed
pkt mannequin opened this issue Jul 22, 2015 · 8 comments
Closed

socket.getaddrinfo(host) doesn't ensure that host.encode() returns a byte string #68872

pkt mannequin opened this issue Jul 22, 2015 · 8 comments
Labels
type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@pkt
Copy link
Mannequin

pkt mannequin commented Jul 22, 2015

BPO 24684
Nosy @vstinner, @Rosuav
Files
  • poc_getaddr.py
  • dont_assert.patch
  • dont_assert_with_test.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 = None
closed_at = <Date 2015-09-11.10:45:32.212>
created_at = <Date 2015-07-22.07:02:51.998>
labels = ['type-crash']
title = "socket.getaddrinfo(host) doesn't ensure that host.encode() returns a byte string"
updated_at = <Date 2015-09-11.10:45:32.211>
user = 'https://bugs.python.org/pkt'

    bugs.python.org fields:

    activity = <Date 2015-09-11.10:45:32.211>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2015-09-11.10:45:32.212>
closer = 'vstinner'
components = []
creation = <Date 2015-07-22.07:02:51.998>
creator = 'pkt'
dependencies = []
files = ['39974', '40434', '40435']
hgrepos = []
issue_num = 24684
keywords = ['patch']
message_count = 8.0
messages = ['247094', '247097', '247098', '247101', '250456', '250457', '250461', '250462']
nosy_count = 4.0
nosy_names = ['vstinner', 'python-dev', 'Rosuav', 'pkt']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue24684'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']
    @pkt
    Copy link
    Mannequin Author

    pkt mannequin commented Jul 22, 2015

    eck(idna));
    # (gdb)
    #
    # Program received signal SIGABRT, Aborted.
    # 0xb77a6d4c in __kernel_vsyscall ()
    #
    # "host" argument can be set to a subclass of unicode with a custom "encode"
    # method. "encode" returns unexpected type. assert is not compiled in release
    # mode, so this will lead to a type confusion later on.

    @pkt pkt mannequin added the type-crash A hard crash of the interpreter, possibly with a core dump label Jul 22, 2015
    @vstinner
    Copy link
    Member

    5513 idna = _PyObject_CallMethodId(hobj, &PyId_encode, "s", "idna");
    5514 if (!idna)
    5515 return NULL;
    5516 assert(PyBytes_Check(idna));

    The assertion fails because the custom string type in poc_getaddr.py returns an integer, not a byte string.

    IMHO we should call PyUnicode_AsEncodedObject() instead of calling the encode() method.

    @vstinner vstinner changed the title Type confusion in socket module socket.getaddrinfo(host) doesn't ensure that host.encode() returns a byte string Jul 22, 2015
    @vstinner
    Copy link
    Member

    @paul: are you fuzzing Python?

    @pkt
    Copy link
    Mannequin Author

    pkt mannequin commented Jul 22, 2015

    @Haypo:
    I'd be happy to implement all my fuzzer ideas if my bugs were patched in a timely manner.

    At this moment I have multiple bugs submitted over 2 months ago, which still aren't patched. Without patches, hackerone won't accept these issues, so my incentive to work on python is removed.

    @Rosuav
    Copy link
    Contributor

    Rosuav commented Sep 11, 2015

    ISTM this is a case where Python's core shouldn't be using assert. It's possible for userland code to trigger an assertion failure, which means it should be a regular if(..) raise. Patch attached.

    @Haypo, what do you mean by "fuzzing"? Is there something I've missed here?

    @Rosuav
    Copy link
    Contributor

    Rosuav commented Sep 11, 2015

    Oops, forgot to add a test. Using a variant of poc_getaddr.py to construct something which fails on current CPython tip, and passes with the patch.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 11, 2015

    New changeset 2bff115e6ba0 by Victor Stinner in branch '3.4':
    Issue bpo-24684: socket.socket.getaddrinfo() now calls
    https://hg.python.org/cpython/rev/2bff115e6ba0

    New changeset 0c13674cf8b5 by Victor Stinner in branch '2.7':
    Issue bpo-24684: socket.socket.getaddrinfo() now calls
    https://hg.python.org/cpython/rev/0c13674cf8b5

    @vstinner
    Copy link
    Member

    Ok, I fixed the bug in Python 2.7, 3.4, 3.5 and 3.6. (Python 2.7 was also impacted for custom *unicode* strings.)

    Thanks for your bug report paul!

    ISTM this is a case where Python's core shouldn't be using assert. It's possible for userland code to trigger an assertion failure, which means it should be a regular if(..) raise.

    Right, this check is implemented in PyUnicode_AsEncodedString(). Moreover, PyUnicode_AsEncodedString() calls directly the codec, it doesn't call the encode() method of the input string.

    (Sorry, I wrote PyUnicode_AsEncodedObject() which has a different purpose.)

    @Haypo, what do you mean by "fuzzing"?

    This: https://en.wikipedia.org/wiki/Fuzz_testing

    @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
    No one assigned
    Labels
    type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @Rosuav