Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Upgrade libuv to v1.48.0 #600

Merged
merged 8 commits into from
Aug 15, 2024
Merged

Upgrade libuv to v1.48.0 #600

merged 8 commits into from
Aug 15, 2024

Conversation

niklasr22
Copy link
Contributor

@niklasr22 niklasr22 commented Mar 12, 2024

Upgrades libuv to v1.48.0 which fixes a security vulnerability.

I removed two DNS test cases because they raise an error intended by libuv.

@niklasr22
Copy link
Contributor Author

I think the pipeline should be able to pass as it did in the PR in my fork repo. Could someone please trigger a retry?

@fantix
Copy link
Member

fantix commented Mar 13, 2024

yeah that error in CI looks like an old flake

@tapple-cisco
Copy link

tapple-cisco commented Jun 21, 2024

I did some investigating about that venerability. I checked if I could reproduce the ‘truncate after 256 bytes’ venerability. I cannot exploit it if I pass hostname as a string, due to this idna encoding line; uvloop accidentally protects you from the libuv venerability:

>>> payload = f'0x{"0"*246}7f000001.example.com'
>>> payload
'0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f000001.example.com'

>>> import uvloop, asyncio
>>> async def loop_getaddrinfo(addr, port): return await asyncio.get_running_loop().getaddrinfo(addr, port)
>>> uvloop.run(loop_getaddrinfo(payload, 80))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/mfulmer/miniconda3/envs/lm2/lib/python3.9/site-packages/uvloop/__init__.py", line 82, in run
    return loop.run_until_complete(wrapper())
  File "uvloop/loop.pyx", line 1517, in uvloop.loop.Loop.run_until_complete
  File "/home/mfulmer/miniconda3/envs/lm2/lib/python3.9/site-packages/uvloop/__init__.py", line 61, in wrapper
    return await main
  File "<stdin>", line 1, in loop_get_addrinfo
  File "uvloop/loop.pyx", line 1528, in getaddrinfo
  File "uvloop/loop.pyx", line 905, in uvloop.loop.Loop._getaddrinfo
UnicodeError: encoding with 'idna' codec failed (UnicodeError: label too long)

This is a similar error that socket gives:

>>> import socket
>>> socket.getaddrinfo(payload, "80")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/mfulmer/miniconda3/envs/lm2/lib/python3.9/socket.py", line 954, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
UnicodeError: encoding with 'idna' codec failed (UnicodeError: label too long)

However, if I pass the hostname as bytes, I can bypass the accidental uvloop protection and exploit libuv:

>>> payload = f'0x{"0"*246}7f000001.example.com'.encode()
>>> payload
b'0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f000001.example.com'
>>> uvloop.run(loop_getaddrinfo(payload, 80))
[(<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('127.0.0.1', 80)), (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_DGRAM: 2>, 17, '', ('127.0.0.1', 80)), (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_RAW: 3>, 0, '', ('127.0.0.1', 80))]

socket, however, isn’t fooled:

>>> socket.getaddrinfo(payload, "80")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/mfulmer/miniconda3/envs/lm2/lib/python3.9/socket.py", line 954, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -2] Name or service not known

I didn’t know about this "0x7f000001" form of hostnames, but, apparently it’s a thing:

>>> socket.getaddrinfo("0x7f000001", "80")
[(<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('127.0.0.1', 80)), (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_DGRAM: 2>, 17, '', ('127.0.0.1', 80)), (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_RAW: 3>, 0, '', ('127.0.0.1', 80))]

I can’t find any documentation about why that’s considered a valid hostname. It’s obviously a hex encoding of a 4-byte ipv4 address, but, I’ve never seen it written that way

Anyway, maybe you can turn my investigation into a unit test for the security venerability

@tapple-cisco
Copy link

tapple-cisco commented Jun 24, 2024

regarding the idna encoding error, there's some discussion of whether that error should be handled a different way in the python standard library or not. Just for reference: python/cpython#77139

This was referenced Aug 14, 2024
fantix added 5 commits August 13, 2024 23:49
This reverts commit 281dc2c.
It seems getaddrinfo('', ...) on macOS is equivalent to nodename='localhost'.
This is inconsistent with libuv 1.48 which treats empty nodename as EINVAL.
Thanks to @tapple-cisco for the repro
@fantix fantix requested review from 1st1 and elprans August 14, 2024 04:46
err = ex

try:
a2 = self.loop.run_until_complete(
self.loop.getaddrinfo(*args, **kwargs))
except socket.gaierror as ex:
except (socket.gaierror, UnicodeError) as ex:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What input would trigger a UnicodeError?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what @tapple-cisco mentioned with the vulnerability repro, as well as the BPO. A short example with CPython is like:

>>> payload = f'0x{"0"*246}7f000001.example.com'
>>> import socket; socket.getaddrinfo(payload, 0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.12/socket.py", line 964, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/encodings/idna.py", line 173, in encode
    raise UnicodeError("label empty or too long")
UnicodeError: label empty or too long
encoding with 'idna' codec failed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is encoding with 'idna' codec failed a context exception?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.e. .__context__.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it's a weird output, let me see

Copy link
Member

@fantix fantix Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's __notes__, this particular note is added in 3.12 (changed from a __context__).

@fantix fantix merged commit 7777852 into MagicStack:master Aug 15, 2024
11 checks passed
fantix added a commit that referenced this pull request Aug 15, 2024
Changes
=======

* Upgrade libuv to v1.48.0 (#600)
  (by @niklasr22 @fantix in 7777852 for #596 #615)

Fixes
=====

* Fix test_create_server_4 with Python 3.12.5 (#614)
  (by @shadchin in 62f9239)

* Use len(os.sched_getaffinity(0)) instead of os.cpu_count() (#591)
  (by @avkarenow in c8531c2 for #591)

* Inline _Py_RestoreSignals() from CPython (#604)
  (by @befeleme in 8511ba1 for #603)
@fantix fantix mentioned this pull request Aug 15, 2024
edgarrmondragon pushed a commit to edgarrmondragon/uvloop that referenced this pull request Aug 19, 2024
* Fix for libuv 1.48
* Fix for macOS (resolve empty host string as "localhost")
* Add test

---------

Co-authored-by: Fantix King <fantix.king@gmail.com>
edgarrmondragon pushed a commit to edgarrmondragon/uvloop that referenced this pull request Aug 19, 2024
Changes
=======

* Upgrade libuv to v1.48.0 (MagicStack#600)
  (by @niklasr22 @fantix in 7777852 for MagicStack#596 MagicStack#615)

Fixes
=====

* Fix test_create_server_4 with Python 3.12.5 (MagicStack#614)
  (by @shadchin in 62f9239)

* Use len(os.sched_getaffinity(0)) instead of os.cpu_count() (MagicStack#591)
  (by @avkarenow in c8531c2 for MagicStack#591)

* Inline _Py_RestoreSignals() from CPython (MagicStack#604)
  (by @befeleme in 8511ba1 for MagicStack#603)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants