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

general_bad_file_permissions permits most dangerous flag #569

Closed
tonybaloney opened this issue Jan 31, 2020 · 3 comments · Fixed by #570
Closed

general_bad_file_permissions permits most dangerous flag #569

tonybaloney opened this issue Jan 31, 2020 · 3 comments · Fixed by #570
Assignees
Labels
enhancement New feature or request

Comments

@tonybaloney
Copy link
Contributor

The logic in general_bad_file_permissions seems to ignore stat.S_IXOTH (execute by others), which is the most dangerous of all the POSIX file flags.

It raises a high severity issue for S_IWOTH (write by others) and it raises a medium severity issue for S_IXGRP (execute by group).

The following test raises no issues:

import os
import stat

os.chmod('/path/to/binary/i/just/uploaded', stat.S_IXOTH)

The guide referenced in the file documents why it is bad to have this flag:
https://security.openstack.org/guidelines/dg_apply-restrictive-file-permissions.html#testing-guide

@lukehinds
Copy link
Member

lukehinds commented Feb 26, 2020

OK, I think there is a fair inclusion. Pycharm also marks the same for S_IXOTH?

https://pycharm-security.readthedocs.io/en/latest/checks/OS100.html

What do you think of adding stat.S_IWGRP group users can write as well @tonybaloney ?

@tonybaloney
Copy link
Contributor Author

Yes, I think stat.S_IWGRP should be included as well.

I can update #570 to have:

  • S_IXGRP or S_IWGRP = medium
  • S_IXOTH or S_IWOTH = high

@lukehinds
Copy link
Member

sounds good, thanks @tonybaloney

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants