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

[pkt]

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]

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.

[vstinner]

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]

@paul: are you fuzzing Python?

[pkt]

@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]

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]

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]

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]

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