Skip to content

Conversation

MidnightRocket
Copy link
Contributor

@MidnightRocket MidnightRocket commented Mar 30, 2025

This prevents a race conditions vulnerability in the make_temp_dir 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.

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

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@MidnightRocket MidnightRocket force-pushed the mktemp/prevent-race-condition-tempdir-permissions branch from b08c611 to 2464ae1 Compare March 31, 2025 06:03
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@MidnightRocket MidnightRocket force-pushed the mktemp/prevent-race-condition-tempdir-permissions branch from 2464ae1 to 7bf478f Compare March 31, 2025 07:15
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

please add a comment in the code, thanks

@MidnightRocket MidnightRocket force-pushed the mktemp/prevent-race-condition-tempdir-permissions branch from 7bf478f to ebb17cb Compare April 1, 2025 11:17
@MidnightRocket
Copy link
Contributor Author

please add a comment in the code, thanks

I Have tried to add what I think could be a relevant comment, any feedback is welcomed 😃.

@MidnightRocket MidnightRocket force-pushed the mktemp/prevent-race-condition-tempdir-permissions branch from ebb17cb to c9e75bc Compare April 3, 2025 22:07
Copy link

github-actions bot commented Apr 4, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

Sorry but please also add a test to verify that the permissions are correctly set

@sylvestre sylvestre force-pushed the mktemp/prevent-race-condition-tempdir-permissions branch from c9e75bc to fdc17c1 Compare April 4, 2025 06:39
Copy link

github-actions bot commented Apr 4, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

@MidnightRocket MidnightRocket force-pushed the mktemp/prevent-race-condition-tempdir-permissions branch from fdc17c1 to df1bd54 Compare April 5, 2025 14:18
@MidnightRocket
Copy link
Contributor Author

MidnightRocket commented Apr 5, 2025

I believe that this test already covers this

Copy link

github-actions bot commented Apr 5, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/printf/printf is no longer failing!

@MidnightRocket MidnightRocket force-pushed the mktemp/prevent-race-condition-tempdir-permissions branch from df1bd54 to 46c93d8 Compare April 8, 2025 14:18
Copy link

github-actions bot commented Apr 8, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

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 MidnightRocket force-pushed the mktemp/prevent-race-condition-tempdir-permissions branch from 46c93d8 to 32fed17 Compare April 9, 2025 09:18
Copy link

github-actions bot commented Apr 9, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@sylvestre sylvestre merged commit 9cb4348 into uutils:main Apr 9, 2025
68 of 69 checks passed
@MidnightRocket MidnightRocket deleted the mktemp/prevent-race-condition-tempdir-permissions branch April 9, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants