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

use 775 default for mkdirs, to avoid world-write #46

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

ashnazg
Copy link
Member

@ashnazg ashnazg commented Jan 28, 2024

For Issue #29. I chose 775 default instead of suggested 755 on the assumption that runtime user's group should be considered safe enough.

@ashnazg ashnazg requested a review from mcdruid January 28, 2024 16:33
@mcdruid
Copy link
Contributor

mcdruid commented Jan 29, 2024

I suppose there's a chance this may break things for some applications that use Archive_Tar, but it seems like a sensible change.

Thanks for adding tests.

@mcdruid mcdruid merged commit 32ef9ea into pear:master Jan 29, 2024
8 checks passed
@ashnazg
Copy link
Member Author

ashnazg commented Jan 29, 2024

I'd do a minor release (1.5.0) on this rather than patch release (1.4.15), based on this behavior change. At least that'll signal to people that something API-level-ish got changed.

@ashnazg
Copy link
Member Author

ashnazg commented Mar 10, 2024

@mcdruid , do you think you'll have time to roll a 1.5.0 release for this soon? If not, I can do it.

@ashnazg ashnazg mentioned this pull request Mar 10, 2024
@mcdruid
Copy link
Contributor

mcdruid commented Mar 11, 2024

@ashnazg yup, I'll work on this in #47

@orlitzky
Copy link

orlitzky commented Jun 10, 2024

In general, mode 0775 will still be insecure (but don't panic, read on). For example, on our web servers, each website user runs as its own account but they share a single websites group. (Having a shared group lets you e.g. chroot by group in sshd_config). Mode 0775 would allow the users/administrators of one site to edit the files of another.

However, I don't think this was your problem to begin with. At the very least, the server administrator should be using umask to set an upper limit on the permissions given to new files and directories. The documentation for the mkdir() function says that the permissions you give it are "modified by the current umask...", and on any reasonable system, the umask will be at least 0022. So even with mkdir(...,0777), the resulting directory will wind up with mode 0755. This is probably why it was 0777 to begin with -- the umask is what actually determines the final set of permission bits.

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.

3 participants