-
Notifications
You must be signed in to change notification settings - Fork 27
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
instchown: operate on fds, not filenames. #266
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who knows, some deployments of notqmail might be done automatically, and better not have some different file used between the chown and chmod. I do not see a TOCTOU though, maybe I missed it...
Does not hurt to have it done with a file descriptor...
Yes, AFAICT it's limited to time-of-chown vs time-of-chmod. But (if I can get it to actually work) as you say, seems wise to not be in position to find out whether it's anything worse. |
9507e1e
to
c9f3e6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I think the automated report it rather pointless and this is no real issue, I actually like that change because it is way more efficient for the OS at the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong checkbox
c9f3e6f
to
dfc9db3
Compare
The previous code had been flagged by CodeQL as "TOCTOU". It's not -- at worst, it's time-of-chown to time-of-chmod -- but this will run more efficiently (and make the warnings go away).
dfc9db3
to
a0ccd53
Compare
The previous code had been flagged by CodeQL as "TOCTOU". It's not -- at worst, it's time-of-chown to time-of-chmod -- but this will run more efficiently (and make the warnings go away).
Tested like so: