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

Security: Create new files without execution permission #2809

Closed
nekr0z opened this issue Oct 28, 2022 · 4 comments
Closed

Security: Create new files without execution permission #2809

nekr0z opened this issue Oct 28, 2022 · 4 comments
Assignees
Labels
bug Something isn't working released Available in the stable release security Impact on server or browser security

Comments

@nekr0z
Copy link

nekr0z commented Oct 28, 2022

Files created by PhotoPrism, such as the imported files ending up in the "originals" directory, album YAMLs under "storage/albums/folder", YAMLs under "storage/sidecar" folder, and many other, are created with executable bit set in permissions.

Steps to reproduce:

  1. Have a photo imported.
  2. Check permissions on the file that appeared in "originals".

Expected behavior:

The created files don't have the executable bit set unless they are executables.

Likely cause of the problem:

destFile, err := os.OpenFile(dest, os.O_RDWR|os.O_CREATE, os.ModePerm)
and
destFile, err := os.OpenFile(dest, os.O_RDWR|os.O_CREATE, os.ModePerm)
(and maybe other places) use os.ModePerm that is 0o777; need to use 0o666, since the file created is not intended to be an executable in either case.

@nekr0z nekr0z added the bug Something isn't working label Oct 28, 2022
@lastzero lastzero added the help wanted Well suited for external contributors! label Oct 28, 2022
@lastzero lastzero changed the title Files created by PhotoPrism have executable bit set Security: Create new files without execution permission Oct 31, 2022
@lastzero lastzero added security Impact on server or browser security and removed help wanted Well suited for external contributors! labels Oct 31, 2022
lastzero added a commit that referenced this issue Oct 31, 2022
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero lastzero self-assigned this Oct 31, 2022
@lastzero lastzero added the please-test Ready for acceptance test label Oct 31, 2022
@lastzero
Copy link
Member

lastzero commented Nov 1, 2022

@nekr0z Would you be able to test this for us? No CLA required 😉

@nekr0z
Copy link
Author

nekr0z commented Nov 1, 2022

@nekr0z Would you be able to test this for us? No CLA required wink

Sure, will give the preview a try as soon as I have time (most likely, on Friday or Saturday), will report back as soon as.

@lastzero lastzero added the released Available in the stable release label Nov 2, 2022
@lastzero lastzero closed this as completed Nov 2, 2022
@nekr0z
Copy link
Author

nekr0z commented Nov 2, 2022

Closed already, but FWIW everything looks good: imports, sidecars, config (I haven't tested album YAMLs yet, but no reason to think they'd behave differently).

@lastzero
Copy link
Member

lastzero commented Nov 2, 2022

Thank you! Yeah, closed, since we already released it and of course tested it ourselves as well.

@lastzero lastzero removed the please-test Ready for acceptance test label Nov 4, 2022
@lastzero lastzero moved this to Released 🌈 in Roadmap 🚀✨ Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released Available in the stable release security Impact on server or browser security
Projects
Status: Release 🌈
Development

No branches or pull requests

2 participants