Skip to content

Conversation

@AZero13
Copy link
Contributor

@AZero13 AZero13 commented Dec 20, 2025

We can literally just use open(path, 'xb+') for _create_carefully

@AZero13 AZero13 requested a review from a team as a code owner December 20, 2025 06:49
@AZero13 AZero13 changed the title gh-143010: Prevent a TOCTOU issue by reusing the fd gh-143010: Prevent a TOCTOU issue by only calling open once Dec 20, 2025
… TOCTOU issue by only calling open once

We can literally just use open(path, 'xb+') for _create_carefully.
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Can you please start with a failing test that can show us what's wrong?

@AZero13
Copy link
Contributor Author

AZero13 commented Dec 20, 2025

Can you please start with a failing test that can show us what's wrong?

This is going to be very difficult given the fact it has to be precisely timed to the nanosecond as it is between opening of the file descriptor to the opening of the path again.

@AZero13
Copy link
Contributor Author

AZero13 commented Dec 21, 2025

By the way, this code is older than the "x" was added in 2012, which is why this wasn't used in the first place.

@bitdancer
Copy link
Member

I think that you can create a test by mocking open with a side_effect that munges things before making the real open call. Is that worth doing?

@bedevere-app
Copy link

bedevere-app bot commented Dec 21, 2025

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.

Co-authored-by: sobolevn <mail@sobolevn.me>
Copy link
Member

@bitdancer bitdancer left a comment

Choose a reason for hiding this comment

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

LGTM

@bitdancer bitdancer merged commit a88d1b8 into python:main Dec 22, 2025
49 of 50 checks passed
@bitdancer bitdancer added awaiting merge needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Dec 22, 2025
@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

Thanks @AZero13 for the PR, and @bitdancer for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 22, 2025
…thonGH-143011)

* pythongh-143010: Prevent a TOCTOU issue by pythongh-143010: Prevent a TOCTOU issue by only calling open once

RDM: per  AZero13's research the 'x' option did not exist when this code was written,  This
modernization can thus drop the fd trick in _create_carefully and just use open with 'x' to achieve the same goal more securely.
(cherry picked from commit a88d1b8)

Co-authored-by: AZero13 <gfunni234@gmail.com>
Co-authored-by: sobolevn <mail@sobolevn.me>
@bedevere-app
Copy link

bedevere-app bot commented Dec 22, 2025

GH-143079 is a backport of this pull request to the 3.13 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 22, 2025
…thonGH-143011)

* pythongh-143010: Prevent a TOCTOU issue by pythongh-143010: Prevent a TOCTOU issue by only calling open once

RDM: per  AZero13's research the 'x' option did not exist when this code was written,  This
modernization can thus drop the fd trick in _create_carefully and just use open with 'x' to achieve the same goal more securely.
(cherry picked from commit a88d1b8)

Co-authored-by: AZero13 <gfunni234@gmail.com>
Co-authored-by: sobolevn <mail@sobolevn.me>
@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Dec 22, 2025
@bedevere-app
Copy link

bedevere-app bot commented Dec 22, 2025

GH-143080 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Dec 22, 2025
@bitdancer
Copy link
Member

While this type of bug is often a security issue, it's hard to see how this could be exploited as one, so I'm not inclined to backport it to the security fix branches. But I'm not part of the security team, so if someone want to overrule me I'm fine with that ;)

bitdancer pushed a commit that referenced this pull request Dec 22, 2025
…H-143011) (#143079)

gh-143010: Prevent a TOCTOU issue by only calling open once (GH-143011)

RDM: per  AZero13's research the 'x' option did not exist when this code was written,  This
modernization can thus drop the fd trick in _create_carefully and just use open with 'x' to achieve the same goal more securely.
(cherry picked from commit a88d1b8)

Co-authored-by: AZero13 <gfunni234@gmail.com>
Co-authored-by: sobolevn <mail@sobolevn.me>
bitdancer pushed a commit that referenced this pull request Dec 22, 2025
…H-143011) (#143080)

gh-143010: Prevent a TOCTOU issue by only calling open once (GH-143011)

RDM: per  AZero13's research the 'x' option did not exist when this code was written,  This
modernization can thus drop the fd trick in _create_carefully and just use open with 'x' to achieve the same goal more securely.
(cherry picked from commit a88d1b8)

Co-authored-by: AZero13 <gfunni234@gmail.com>
Co-authored-by: sobolevn <mail@sobolevn.me>
@AZero13 AZero13 deleted the stat branch December 22, 2025 18:15
@AZero13
Copy link
Contributor Author

AZero13 commented Dec 22, 2025

While this type of bug is often a security issue, it's hard to see how this could be exploited as one, so I'm not inclined to backport it to the security fix branches. But I'm not part of the security team, so if someone want to overrule me I'm fine with that ;)

Let's be honest, this change has more benefits than just preventing a TOCTOU. I should have mentioned that, but hopefully that's also self evident.

@AZero13
Copy link
Contributor Author

AZero13 commented Dec 22, 2025

However, this was the easiest one to make a GitHub issue out of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants