View Issue Details

IDProjectCategoryView StatusLast Update
0003771Composrcore_upgraderpublic2019-12-15 02:52
ReporterChris GrahamAssigned To 
Severityfeature 
Status non-assignedResolutionopen 
Product Version 
Fixed in Version 
Summary0003771: Better "excessive file permissions" detection
DescriptionThe 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.
Additional InformationI 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())) {
TagsRoadmap: v11
Attach Tags
Time estimation (hours)4
Sponsorship open

Activities

Chris Graham

2019-12-15 02:52

administrator   ~0006236

There's a PHP bug where open_basedir errors are incorrectly flagged, for what is actually something else entirely: https://bugs.php.net/bug.php?id=52065. This caused me to start looking at this issue early, as it's even more important to make correct permissions easier to understand and maintain if PHP is going to misreport things.

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

Issue History

Date Modified Username Field Change
2019-02-07 16:13 Chris Graham New Issue
2019-06-27 19:01 Chris Graham Tag Attached: Roadmap: v11
2019-12-15 02:51 Guest Note Added: 0006235
2019-12-15 02:52 Chris Graham Note Added: 0006236