bpo-44493: Add missing terminated NUL in sockaddr_un's length #26866

zonyitoo · 2021-06-23T03:09:21Z

Linux: https://man7.org/linux/man-pages/man7/unix.7.html
*BSD: SUN_LEN

https://bugs.python.org/issue44493

Automerge-Triggered-By: GH:gpshead

the-knights-who-say-ni · 2021-06-23T03:09:24Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@zonyitoo

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

github-actions · 2021-07-26T00:04:14Z

This PR is stale because it has been open for 30 days with no activity.

zonyitoo · 2021-07-26T03:41:22Z

Can anyone review this PR?

zonyitoo · 2021-10-16T06:24:50Z

Can anyone review this PR?

habnabit · 2022-03-23T19:21:52Z

I too am now facing this same issue. How can it be moved along?

habnabit · 2022-03-24T00:11:31Z

I can't believe this missed the 3.10 release. :(

zonyitoo · 2022-03-24T01:27:08Z

Where are the maintainers?

Hello @zooba , @asvetlov , @tiran

tiran · 2022-03-24T08:29:16Z

I'm temporarily locking this PR to prevent further abuse.

The issue on BPO hasn't been triaged because it lacks any sort of description of the issue. You should provide an explanation why you consider the code to be wrong. A reproducer is greatly appreciated. The change set also needs a test case.

PS: In case you wonder where the maintainers are, two are currently in the middle of a war zone and the others are worried for their lives.

serhiy-storchaka

LGTM.

I think it is difficult to write any tests for this, right?

Modules/socketmodule.c

gpshead

A regression test seems doable and potentially important if I understand this bug correctly:

It looks like the length would be short by one in Python before this change, meaning binding to a AF_UNIX socket potentially loses the last character of the path name intended to be bound?

That should be an observable behavior change.

It also suggests that fixing this will break code that has been working around this bug forever by adding an extra character when binding or connecting to a non-anonymous AF_UNIX socket?

bedevere-bot · 2022-03-24T18:28:45Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

miss-islington · 2022-03-27T20:22:25Z

Thanks @zonyitoo for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

gpshead · 2022-03-27T20:22:44Z

our bots have some known glitches :) this PR was good.

bedevere-bot · 2022-03-27T20:23:03Z

GH-32140 is a backport of this pull request to the 3.10 branch.

bedevere-bot · 2022-03-27T20:23:08Z

GH-32141 is a backport of this pull request to the 3.9 branch.

…GH-26866) Add missing terminated NUL in sockaddr_un's length - Linux: https://man7.org/linux/man-pages/man7/unix.7.html - *BSD: SUN_LEN (cherry picked from commit f6b3a07) Co-authored-by: ty <zonyitoo@users.noreply.github.com>

gpshead · 2022-03-27T20:25:47Z

regarding the conditional defines and sun_len population, if there is a problem being caused by not doing that, file a separate issue and it can be addressed via that.

miss-islington · 2022-03-27T21:43:31Z

Thanks @zonyitoo for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

…H-26866) (GH-32140) Add missing terminated NUL in sockaddr_un's length - Linux: https://man7.org/linux/man-pages/man7/unix.7.html - *BSD: SUN_LEN (cherry picked from commit f6b3a07) Co-authored-by: ty <zonyitoo@users.noreply.github.com> Automerge-Triggered-By: GH:gpshead

…ythonGH-26866) (pythonGH-32140) Add missing terminated NUL in sockaddr_un's length - Linux: https://man7.org/linux/man-pages/man7/unix.7.html - *BSD: SUN_LEN (cherry picked from commit f6b3a07) Co-authored-by: ty <zonyitoo@users.noreply.github.com> Automerge-Triggered-By: GH:gpshead (cherry picked from commit 5944807) Co-authored-by: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com>

…H-26866) (GH-32140) (GH-32156) Add missing terminated NUL in sockaddr_un's length - Linux: https://man7.org/linux/man-pages/man7/unix.7.html - *BSD: SUN_LEN (cherry picked from commit f6b3a07) Co-authored-by: ty <zonyitoo@users.noreply.github.com> Automerge-Triggered-By: GH:gpshead (cherry picked from commit 5944807) Co-authored-by: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com>

…ythonGH-26866) (pythonGH-32140) (pythonGH-32156) Add missing terminated NUL in sockaddr_un's length - Linux: https://man7.org/linux/man-pages/man7/unix.7.html - *BSD: SUN_LEN (cherry picked from commit f6b3a07) Co-authored-by: ty <zonyitoo@users.noreply.github.com> Automerge-Triggered-By: GH:gpshead (cherry picked from commit 5944807) Co-authored-by: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com>

When binding a unix socket to an empty address on Linux, the socket is automatically bound to an available address in the abstract namespace. >>> s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) >>> s.bind("") >>> s.getsockname() b'\x0075499' Since python 3.9, the socket is bound to the one address: >>> s.getsockname() b'\x00' And trying to bind multiple sockets will fail with: Traceback (most recent call last): File "/home/nsoffer/src/cpython/Lib/test/test_socket.py", line 5553, in testAutobind s2.bind("") OSError: [Errno 98] Address already in use Added 2 tests: - Auto binding empty address on Linux - Failing to bind an empty address on other platforms Fixes f6b3a07 (bpo-44493: Add missing terminated NUL in sockaddr_un's length (GH-26866)

…4826) When binding a unix socket to an empty address on Linux, the socket is automatically bound to an available address in the abstract namespace. >>> s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) >>> s.bind("") >>> s.getsockname() b'\x0075499' Since python 3.9, the socket is bound to the one address: >>> s.getsockname() b'\x00' And trying to bind multiple sockets will fail with: Traceback (most recent call last): File "/home/nsoffer/src/cpython/Lib/test/test_socket.py", line 5553, in testAutobind s2.bind("") OSError: [Errno 98] Address already in use Added 2 tests: - Auto binding empty address on Linux - Failing to bind an empty address on other platforms Fixes f6b3a07 (bpo-44493: Add missing terminated NUL in sockaddr_un's length (pythonGH-26866) (cherry picked from commit c22f134) Co-authored-by: Nir Soffer <nsoffer@redhat.com>

When binding a unix socket to an empty address on Linux, the socket is automatically bound to an available address in the abstract namespace. >>> s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) >>> s.bind("") >>> s.getsockname() b'\x0075499' Since python 3.9, the socket is bound to the one address: >>> s.getsockname() b'\x00' And trying to bind multiple sockets will fail with: Traceback (most recent call last): File "/home/nsoffer/src/cpython/Lib/test/test_socket.py", line 5553, in testAutobind s2.bind("") OSError: [Errno 98] Address already in use Added 2 tests: - Auto binding empty address on Linux - Failing to bind an empty address on other platforms Fixes f6b3a07 (bpo-44493: Add missing terminated NUL in sockaddr_un's length (GH-26866) (cherry picked from commit c22f134) Co-authored-by: Nir Soffer <nsoffer@redhat.com>

) When binding a unix socket to an empty address on Linux, the socket is automatically bound to an available address in the abstract namespace. >>> s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) >>> s.bind("") >>> s.getsockname() b'\x0075499' Since python 3.9, the socket is bound to the one address: >>> s.getsockname() b'\x00' And trying to bind multiple sockets will fail with: Traceback (most recent call last): File "/home/nsoffer/src/cpython/Lib/test/test_socket.py", line 5553, in testAutobind s2.bind("") OSError: [Errno 98] Address already in use Added 2 tests: - Auto binding empty address on Linux - Failing to bind an empty address on other platforms Fixes f6b3a07 (bpo-44493: Add missing terminated NUL in sockaddr_un's length (GH-26866) (cherry picked from commit c22f134) Co-authored-by: Nir Soffer <nsoffer@redhat.com>

the-knights-who-say-ni added the CLA not signed label Jun 23, 2021

bedevere-bot added the awaiting review label Jun 23, 2021

zonyitoo mentioned this pull request Jun 23, 2021

SocketAddr from UnixDatagram recv_from missing the last character (on OS X) rust-lang/rust#69061

Closed

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Jun 25, 2021

github-actions bot added the stale Stale PR or inactive for long period of time. label Jul 26, 2021

zonyitoo force-pushed the main branch 2 times, most recently from 9d52974 to f6c4adc Compare July 26, 2021 02:47

zonyitoo force-pushed the main branch 3 times, most recently from d96470f to 75a4ce9 Compare July 26, 2021 05:52

github-actions bot removed the stale Stale PR or inactive for long period of time. label Jul 27, 2021

This comment was marked as abuse.

Sign in to view

python locked as too heated and limited conversation to collaborators Mar 24, 2022

serhiy-storchaka approved these changes Mar 24, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Mar 24, 2022

serhiy-storchaka reviewed Mar 24, 2022

View reviewed changes

Modules/socketmodule.c Outdated Show resolved Hide resolved

python unlocked this conversation Mar 24, 2022

gpshead requested changes Mar 24, 2022

View reviewed changes

bedevere-bot removed the awaiting merge label Mar 24, 2022

gpshead merged commit f6b3a07 into python:main Mar 27, 2022

bedevere-bot removed the awaiting merge label Mar 27, 2022

bedevere-bot removed the needs backport to 3.10 only security fixes label Mar 27, 2022

bedevere-bot removed the needs backport to 3.9 only security fixes label Mar 27, 2022

gpshead added the needs backport to 3.9 only security fixes label Mar 27, 2022

zonyitoo mannequin mentioned this pull request Jul 13, 2022

Missing terminated NUL in the length of sockaddr_un #88659

Closed

nirs mentioned this pull request Jul 13, 2022

gh-94821: Fix autobind of empty unix domain address #94826

Merged

miss-islington mentioned this pull request Jul 15, 2022

[3.11] gh-94821: Fix autobind of empty unix domain address (GH-94826) #94873

Merged

miss-islington mentioned this pull request Jul 15, 2022

[3.10] gh-94821: Fix autobind of empty unix domain address (GH-94826) #94874

Merged

miss-islington mentioned this pull request Jul 15, 2022

[3.9] gh-94821: Fix autobind of empty unix domain address (GH-94826) #94875

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-44493: Add missing terminated NUL in sockaddr_un's length #26866

bpo-44493: Add missing terminated NUL in sockaddr_un's length #26866

zonyitoo commented Jun 23, 2021 •

edited by miss-islington

Loading

the-knights-who-say-ni commented Jun 23, 2021

github-actions bot commented Jul 26, 2021

zonyitoo commented Jul 26, 2021

zonyitoo commented Oct 16, 2021

habnabit commented Mar 23, 2022

habnabit commented Mar 24, 2022

zonyitoo commented Mar 24, 2022 •

edited

Loading

This comment was marked as abuse.

tiran commented Mar 24, 2022 •

edited

Loading

serhiy-storchaka left a comment

gpshead left a comment

bedevere-bot commented Mar 24, 2022

miss-islington commented Mar 27, 2022

gpshead commented Mar 27, 2022

bedevere-bot commented Mar 27, 2022

bedevere-bot commented Mar 27, 2022

gpshead commented Mar 27, 2022

miss-islington commented Mar 27, 2022

bpo-44493: Add missing terminated NUL in sockaddr_un's length #26866

bpo-44493: Add missing terminated NUL in sockaddr_un's length #26866

Conversation

zonyitoo commented Jun 23, 2021 • edited by miss-islington Loading

the-knights-who-say-ni commented Jun 23, 2021

CLA Missing

github-actions bot commented Jul 26, 2021

zonyitoo commented Jul 26, 2021

zonyitoo commented Oct 16, 2021

habnabit commented Mar 23, 2022

habnabit commented Mar 24, 2022

zonyitoo commented Mar 24, 2022 • edited Loading

This comment was marked as abuse.

tiran commented Mar 24, 2022 • edited Loading

serhiy-storchaka left a comment

Choose a reason for hiding this comment

gpshead left a comment

Choose a reason for hiding this comment

bedevere-bot commented Mar 24, 2022

miss-islington commented Mar 27, 2022

gpshead commented Mar 27, 2022

bedevere-bot commented Mar 27, 2022

bedevere-bot commented Mar 27, 2022

gpshead commented Mar 27, 2022

miss-islington commented Mar 27, 2022

zonyitoo commented Jun 23, 2021 •

edited by miss-islington

Loading

zonyitoo commented Mar 24, 2022 •

edited

Loading

tiran commented Mar 24, 2022 •

edited

Loading