gh-143010: Prevent a TOCTOU issue by only calling open once#143011
Conversation
… TOCTOU issue by only calling open once We can literally just use open(path, 'xb+') for _create_carefully.
sobolevn
left a comment
There was a problem hiding this comment.
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. |
|
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. |
|
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? |
|
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 |
Co-authored-by: sobolevn <mail@sobolevn.me>
|
Thanks @AZero13 for the PR, and @bitdancer for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Thanks @AZero13 for the PR, and @bitdancer for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…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>
|
GH-143079 is a backport of this pull request to the 3.13 branch. |
…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>
|
GH-143080 is a backport of this pull request to the 3.14 branch. |
|
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 ;) |
…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>
…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>
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. |
|
However, this was the easiest one to make a GitHub issue out of. |
We can literally just use open(path, 'xb+') for _create_carefully
mailbox._create_carefullyhas a possible toctou error #143010