mktemp: Prevent race condition when setting permissions for tempdir #7617
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
GNU testsuite comparison: |
MidnightRocket
force-pushed
the
mktemp/prevent-race-condition-tempdir-permissions
branch
from
b08c611 to
2464ae1
Compare
|
GNU testsuite comparison: |
MidnightRocket
force-pushed
the
mktemp/prevent-race-condition-tempdir-permissions
branch
from
2464ae1 to
7bf478f
Compare
|
GNU testsuite comparison: |
Contributor
|
please add a comment in the code, thanks |
MidnightRocket
force-pushed
the
mktemp/prevent-race-condition-tempdir-permissions
branch
from
7bf478f to
ebb17cb
Compare
Contributor
Author
I Have tried to add what I think could be a relevant comment, any feedback is welcomed 😃. |
MidnightRocket
force-pushed
the
mktemp/prevent-race-condition-tempdir-permissions
branch
from
ebb17cb to
c9e75bc
Compare
|
GNU testsuite comparison: |
Contributor
|
Sorry but please also add a test to verify that the permissions are correctly set |
sylvestre
force-pushed
the
mktemp/prevent-race-condition-tempdir-permissions
branch
from
c9e75bc to
fdc17c1
Compare
|
GNU testsuite comparison: |
MidnightRocket
force-pushed
the
mktemp/prevent-race-condition-tempdir-permissions
branch
from
fdc17c1 to
df1bd54
Compare
Contributor
Author
|
I believe that this test already covers this |
|
GNU testsuite comparison: |
MidnightRocket
force-pushed
the
mktemp/prevent-race-condition-tempdir-permissions
branch
from
df1bd54 to
46c93d8
Compare
|
GNU testsuite comparison: |
This prevents a race conditions vulnerability in the tempdir implementation, where an attacker potentially could modify the created temporary directory, before the restrictive permissions are set. The race conditions occurs in the moment between the temporary directory is created, and the proper permissions are set. # The fix This patch changes the `make_temp_dir` to create the temporary directory with the proper permissions creation time. Rather than first create, then set permissions. This is done by giving the permissions to the builder. See [tempfile doc](https://github.com/Stebalien/tempfile/blob/95540ed3fcb9ca74845c02aee058726b2dca58b7/src/lib.rs#L449-L450). # Severity Low The attack is only possible if the umask is configured to allow writes by group or other for created file/directories. # Related Resources See: https://cwe.mitre.org/data/definitions/377.html
MidnightRocket
force-pushed
the
mktemp/prevent-race-condition-tempdir-permissions
branch
from
46c93d8 to
32fed17
Compare
|
GNU testsuite comparison: |
MidnightRocket
deleted the
mktemp/prevent-race-condition-tempdir-permissions
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This prevents a race conditions vulnerability in the
make_temp_dirimplementation, where an attacker potentially could modify the created temporary directory, before the restrictive permissions are set.The race conditions occurs in the moment between the temporary directory is created, and the proper permissions are set.
The fix
This patch changes the
make_temp_dirto create the temporary directory with the proper permissions creation time. Rather than first create, then set permissions. This is done by giving the permissions to the builder. See tempfile doc.Severity Low
The attack is only possible if the umask is configured to allow writes by group or other for newly created files/directories.
Related Resources
See: https://cwe.mitre.org/data/definitions/377.html