#3771 - Better "excessive file permissions" detection
-
By Chris Graham
- Added
- 8 views
| Identifier | #3771 |
|---|---|
| Issue type | Feature request or suggestion |
| Title | Better "excessive file permissions" detection |
| Status | Completed |
| Tags |
Roadmap: v11 (custom) |
| Handling member | Chris Graham |
| Addon | core_upgrader |
| Description | The excessive file permissions checker currently only checks when non-suEXEC servers have files/directories chmodded as world-writable that don't need to be (hence lowering security as any other web server user may potentially have write access).
Actually there's a more important check we should do. For suEXEC servers, find any files/directories that are world-writable - none should be. Some Apache servers will give 500 errors if PHP files being called up are. Really we might want to approach this in an absolutist way - knowing, for a particular server's architecture, what every files permissions should be - and correcting it to that. There's no need for example to have executable set on PHP files, unless the server needs that. |
| Steps to reproduce | |
| Additional information | I changed the current test line for a user, to hack the main new use case described here, and it worked...
if ((php_function_allowed('posix_getuid')) && ((fileperms($dir . $file) & 2) != 0) && (fileowner($dir . $file) == posix_getuid())) { |
| Funded? | No |
The system will post a comment when this issue is modified (e.g., status changes). To be notified of this, click "Enable comment notifications".
Comments
I've done most of what is required in a new class I've committed called CMSPermissionsScanner. This class can run standalone of Composr, which is important because if permissions are broken Composr might not be runnable.
It checks regular permissions in an almost absolutist way, it can check extended attributes, and it can check selinux.
I want to go beyond the original scope of this issue and make this class the consistent back-bone of all Composr permission checks/fixup:
- fixperms.sh and fixperms.bat become front-ends to the class, running the commands it outputs
- these scripts will now take an optional parameter that specifies the username of the web user (currently they assume they need to set things as world-writable)
- for fixperms.sh that will default to apache or nobody or http or httpd, whatever exists
- for fixperms.bat that will default to IUSR
- security is important, they can't just front-end to code that could also be run over a web URL - any code must be initiated either using "php -r", or do an "is_cli" check
- upgrader permission checks / fixup (for those with no shell access but may have a broken site)
- Health Check permission check (the ideal place for checking everything)
- If some kind of permission check exists for Commandr, remove it
Any unit tests that check that our permissions checking is in sync everywhere can be simplified, as now the only place listing permissions will be inst_special.php and the documentation.
Improvements to the class:
- Add ability to spit out Windows permission fixing commands (icacls not chmod); test it works as expected on Windows