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

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.

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.

@paul: are you fuzzing Python?

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

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?

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.

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

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

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

versions: + Python 2.7

messages: + msg250457

nosy: + Rosuav
messages: + msg250456

keywords: + patch

versions: + Python 3.4, Python 3.6