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

mkdir default mask #29

Closed
beat opened this issue Jan 9, 2020 · 5 comments
Closed

mkdir default mask #29

beat opened this issue Jan 9, 2020 · 5 comments
Assignees

Comments

@beat
Copy link

beat commented Jan 9, 2020

In these 2 lines, a mask of 0777 is used to create directories. This flags some PHP source code security scanners. A safer suggested value here would be 0755:

if (!@mkdir($v_header['filename'], 0777)) {

if (!@mkdir($p_dir, 0777)) {

@rgpublic
Copy link

rgpublic commented Sep 4, 2020

0777 is the default value anyway so could be omitted altogether - let's see whether the dumb security scanner catches THAT ;-) Joking aside, just omitting is IMHO really the best option instead of taking any sides - if people want to influence that they could just as well set a default umask.

@ashnazg
Copy link
Member

ashnazg commented Dec 9, 2023

@mrook , do you recall any regular runtime issues that necessitated using 0777 on these mkdir() calls? If you don't remember anything specific, I might look at testing out adjusting it to 0755, on the assumption that it should work fine for the runtime user, and the userland code would still have the flexibility to modify the resulting files anyway if needed.

Just looking for some guidance from your experience :)

@mrook
Copy link
Member

mrook commented Dec 11, 2023

@ashnazg no the mkdir(xxx, 0777) calls have been in the code since well before I became active on the project. It seems it was already in the code when the package was still experimental: ea8b039

@ashnazg
Copy link
Member

ashnazg commented Jan 7, 2024

Thanks much @mrook ... that takes some fear away from me regarding this change :)

@mcdruid, I'll assign this to you... that way you can use this opportunity to do a PR and run through the overall release process for the package. I'd probably make this a point release rather than patch release, given the potential of the visible runtime behavior change.

@ashnazg
Copy link
Member

ashnazg commented Mar 10, 2024

Fixed on #46... should be released as 1.5.0 soon.

@ashnazg ashnazg closed this as completed Mar 10, 2024
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

No branches or pull requests

5 participants