Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install: create destination file with safer modes before copy #6595

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nerdroychan
Copy link

Fix #6435.

As is suggested by the original issue, the creation of the destination file is delegated to fs::copy which creates the file with default mode 0644. This makes the file readable by other users before the final chmod is done, which is potentially unsafe and inconsistent with GNU install.

Since the current implementation unconditionally removes the destination path before the copy, creating the file with a minimal u=rw permission would not hurt the copy. This patch implements this (for unix).

@sylvestre
Copy link
Sponsor Contributor

Could you please add a test to make sure we don't regress? Thanks

@nerdroychan nerdroychan force-pushed the install_copy_permission branch 3 times, most recently from 11830cd to 2eb2c4c Compare July 27, 2024 23:13
@nerdroychan
Copy link
Author

Could you please add a test to make sure we don't regress? Thanks

There was indeed a regression: when the fs::copy call failed, the created destination file was not removed. I just pushed a new patch that fixes this.

I added a test to make sure that no file will be created when fs::copy fails (e.g., the source file cannot be read), and if it succeed, the destination file's mode is set correctly. This makes sure that it works as before no matter the copy succeeds or not.

I'm not sure how to use a test case to check whether the new file is created in mode 0600, since the creation is in the middle of an internal function, but the strace output confirms so If you have any suggestions please let me know. Thanks!

@nerdroychan nerdroychan force-pushed the install_copy_permission branch 3 times, most recently from c9ded01 to 5b9c0c6 Compare July 28, 2024 17:08
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

Copy link

github-actions bot commented Sep 5, 2024

GNU testsuite comparison:

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

Copy link

GNU testsuite comparison:

GNU test failed: tests/timeout/timeout. tests/timeout/timeout is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

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.

install: insecure mode handling
2 participants