J3 Issue No Code Attached Yet
Referenced as Duplicate of: # 25268
avatar sven420
sven420
10 Jan 2019

When Path::canChmod($file) returns false method throws an exception as if deletion is not possible but some servers have such setup that chmod is not but file deletion is possible.

Because of this I got errors when trying to update extensions but after commenting out this and line below that is trying to chmod file to 0777 update was success.

In my opinion unlink($file) should be executed even if chmod is not possible and this is a bug that can cause issues for some users.

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
4.00

avatar sven420 sven420 - open - 10 Jan 2019
avatar joomla-cms-bot joomla-cms-bot - change - 10 Jan 2019
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 10 Jan 2019
avatar mbabker
mbabker - comment - 10 Jan 2019

In my opinion unlink($file) should be executed even if chmod is not possible and this is a bug that can cause issues for some users.

-1, the entire point of attempting to chmod the resource is to make sure it is writable as a delete action is a write action. Blindly executing chmod() without ensuring the resource is in a writable state or executing unlink() on a read only resource is also prone to errors, removing sanity checks is not a valid bug fix here.

The fact you are having issues with the chmod() function points to either PHP being too restricted (i.e. the function is in the disabled functions list), filesystem permissions oddities, or the umask being an invalid value causing chmod(0777) to put the file into a read-only state.

avatar sven420
sven420 - comment - 10 Jan 2019

I see your point but we are not always in the situation where we have control over PHP or server configuration and Joomla should work in those cases also.

I don't think chmod is good sanity check here because executing unlink in try catch block would produce exception in case file isn't writeable.

What if chmod passes but unlink fails for some reason, if chmod check is definite proof that file is writable another try catch block for unlink is not necessary.

I still think that unlink try catch should be executed even if chmod part fails, there is no reason not to try it in method named delete.

avatar mbabker
mbabker - comment - 10 Jan 2019

I still think that unlink try catch should be executed even if chmod part fails, there is no reason not to try it in method named delete.

If anything is going to change about the code, this should be the workflow. It should continue to be full of sanity checks, but potentially gets around whatever issue you might be running into. At the end of the day, the API MUST ensure it can actually work with the file given otherwise everyone's better off just hardcoding the use of unlink() into their code.

  • Check file actually exists, if not then return true (because it should never be considered an error to attempt to delete a non-existing filesystem resource, you wanted it gone and it's not there)
  • Check file is writable, if not then execute chmod($file, 0777); and fail if chmod() cannot be performed
  • Attempt to delete file via unlink(), if this fails and chmod() was executed attempt to restore to the previous state
avatar sven420
sven420 - comment - 10 Jan 2019

I agree this is the proper way to do this.

avatar rotech1
rotech1 - comment - 15 Jan 2019

Recently have similar issues with an extension update ( and un-install, too! ) see https://forum.joomla.org/viewtopic.php?f=728&t=968897


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23504.

avatar mbabker
mbabker - comment - 15 Jan 2019

For the record nothing significant has changed in Joomla\CMS\Filesystem\File (previously JFile) nor Joomla\Filesystem\File in quite some time. There were exactly zero changes in those classes between 3.9.0 and 3.9.1 (or for that fact any other parts of the code using methods from those classes) so if something has changed between those two releases it has nothing to do with the filesystem API.

Without a consistent environment to debug the root cause of the issue, at best all we're getting in this thread is "this code is blocking me from doing something" and that doesn't establish why it's preventing you from doing something.

avatar rotech1
rotech1 - comment - 15 Jan 2019

It really looks like 3.9.2 has some kind of filesystem bug - see also https://www.kunena.org/forum/k5-1-support/156312-j3-9-2-breaks-attachment-uploads


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23504.

avatar mbabker
mbabker - comment - 15 Jan 2019

You can't have a new bug in an API that had zero changes in 3 releases.

avatar rotech1
rotech1 - comment - 15 Jan 2019

Well, some very file-system-like problems suddenly appear after the J3.9.2 update - what else could be causing these behaviors?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23504.

avatar mbabker
mbabker - comment - 15 Jan 2019

Without an environment to recreate those issues I couldn't tell you. I can tell you though that the filesystem APIs have not been changed in quite some time (independently auditable; https://github.com/joomla/joomla-cms/commits/3.8.13/libraries/joomla/filesystem shows all changes to the libraries/joomla/filesystem folder from 3.8.13 and earlier and https://github.com/joomla/joomla-cms/commits/3.9.2/libraries/src/Filesystem for changes after the classes were namespaced during 3.9's development). So whatever issue you are seeing that points to an error coming from the filesystem API is a symptom of something else.

Again, the first problem is identifying an environment that can actually create the problem. Because if we are going strictly from my own hosting, Joomla's hosting, or my personal development space, I could say this is not a core bug and close it right now as a local environment issue and leave it to you to debug. If there is some sort of core issue though, it's better to troubleshoot and identify it instead of blindly making changes to files that have gone untouched in a substantial amount of time.

avatar mahagr
mahagr - comment - 21 Jan 2019

@mbabker FYI

We are also getting reports where updating/removing extensions fail in Joomla 3.9 but not in Joomla 3.8. I am trying to get more information about the environments as I have no idea what could cause it to happen.

PHP documentation says:

Note: When safe mode is enabled, PHP checks whether the files or directories you are about to operate on have the same UID (owner) as the script that is being executed. In addition, you cannot set the SUID, SGID and sticky bits.

Maybe safe mode is what causes all the issues? Because of the above paragraph in the docs, I don't believe that chmod() is a reliable way to figure out if the file can be deleted or not.

avatar mbabker
mbabker - comment - 21 Jan 2019

If someone wants to start blindly changing code by all means go for it. All this thread has is a bunch of "this is broken and needs fixing" comments but there is still no real issue identified, just a bunch of hypotheticals. That's still not a good way to troubleshoot and fix potential code issues in methods which have not changed but at this point I'm done arguing for proper debugging and troubleshooting of issues.

avatar mahagr
mahagr - comment - 21 Jan 2019

@mbabker I've never proposed blindly to change the code, I guess that we're all just trying to narrow down what could cause the issue to happen -- and why you won't get the same issue with older versions of Joomla.

I just received a response from one user and it looks like he had libraries folder with UID root, which is obviously a permission issue in his end. I have no idea how older Joomla versions do not fail in this case, maybe older Joomla releases just silently fail to update the files?

For a long time, I have suspected that Joomla has a bug in the installer, which sometimes causes files not to be updated even if installation succeeds without an error. Deleting the files manually and installing the same version again always helped before and I believe the same is true now.

Maybe someone finally found the issue and fixed it?

I will let you know if something new comes out on this.

avatar brianteeman
brianteeman - comment - 21 Jan 2019

Safe mode was removed in php 5.4.0 so if that is the problem there is an obvious and easy answer

avatar rytechsites
rytechsites - comment - 23 Jan 2019

We manage over 100 joomla sites, we are on:
Php 7.0.7

We have found for SOME tools (not all)

  • joomlashack's tools (osmap)
  • docman

If we are on release 3.9.2, and we try to upgrade either of these, we end up getting file errors on the 'uninstall of files', it is as if the files don't uninstall properly.

If we manually install joomla 3.8.13, and attempt to run these upgrades everything works perfectly well.

Please note, on my 'privately hosted sites' on siteground I have NOT had this issue at all.

Our servers are 'local' at our university, and they are managed here 100%. Initially we tried to change file permissions, etc, but that didn't work.

Our best workaround was to go back to 3.8.13...

Is there anything else that I could give to help you see the recreation of this issue?

I'm guessing it is a combination of our file system, php and joomla 3.9.2...something...

thanks,
Laura

avatar billtomczak
billtomczak - comment - 23 Jan 2019

@rytechsites I can confirm that we've identified this as the source of problems we've been seeing with our stable of Joomlashack extensions. Updates to library extensions are handled differently in Joomla. First they are uninstalled, then installed. If the uninstall fails - as it does under certain specific conditions - the entire install/upgrade procedure fails leaving the filesystem and/or database corrupted. Untangling the mess is sometimes pretty painful.

So far we've found that it seems to become an issue when an installed site is moved from one server/host to another. But not always, not reliably.

We're in the process of redoing our whole build/install/deployment chain and it will include workarounds for this problem. In the meantime, all we've been able to do is respond to users when it rears its ugly little head.

I'm not entirely convinced it's a problem as such in Joomla code. Definitely some weird quirk of file permission reporting on some servers under some (unspecified/unknown) circumstances.

avatar rytechsites
rytechsites - comment - 23 Jan 2019

Thank you very much!!! I agree, I think it is a strange combination of 'things', because unfortunately it doesn't happen 'all of the time'.

If you need us to run any kind of testing in our environment, because THERE it does happen 100% of the time with specific extensions, please let me know.

thanks,
Laura

avatar rytechsites
rytechsites - comment - 23 Jan 2019

I just realized something... when we 'install our upgrades', we copy our files from our 'production environment' over to our 'development' environment...install all upgrades, then 'move everything back to production'...

which means we do move the files from one host to another...

-- Laura

avatar brianteeman
brianteeman - comment - 23 Jan 2019

We're in the process of redoing our whole build/install/deployment chain and it will include workarounds for this problem. In the meantime, all we've been able to do is respond to users when it rears its ugly little head.

That's why problems never get fixed. People like yourself with the skills to fix it never do. You just keep it to yourselves.

And of course you don't even need to work around the problem because @mbabker already fixed it #23303

avatar mahagr
mahagr - comment - 24 Jan 2019

And of course you don't even need to work around the problem because @mbabker already fixed it #23303

Well, not quite true. The fix for the library installer is already present in J3.9.2 and it doesn't have any effect on this chmod issue. That said, I'm really glad to see the library installer fix!

But in this case, it is better to fail with a nasty error message than to silently ignore files which cannot be updated. It is far better to have the installer to fail than to end up having a wrong set of files.

@rytechsites If you copy files from one server to another, make sure that the file owner/group are correct for all files after doing that. The installer won't be able to copy/replace the files if the files are owned by one user while the installer (web server) runs as some other user. :) The only reason why chmod() would fail, is that the user doesn't have the permissions needed to do the operation.

avatar rytechsites
rytechsites - comment - 24 Jan 2019

@mahagr thank you for your notes. We will verify, but as far as I know our 'process' of moving the files from one server to another works as it should.

This was a complicated issue, and I appreciate all involved that are taking the time an anyway they can to assist.

avatar joomla-cms-bot joomla-cms-bot - close - 4 Mar 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 4 Mar 2019
Status New Closed
Closed_Date 0000-00-00 00:00:00 2019-03-04 12:16:52
Closed_By franz-wohlkoenig
avatar joomla-cms-bot joomla-cms-bot - change - 4 Mar 2019
Closed_By franz-wohlkoenig joomla-cms-bot
avatar joomla-cms-bot joomla-cms-bot - close - 4 Mar 2019
avatar joomla-cms-bot
joomla-cms-bot - comment - 4 Mar 2019
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 4 Mar 2019

closed as Issue seems resolved. If not please reopen.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23504.

avatar jobst
jobst - comment - 28 Mar 2019

Joomla Version: 3.9.4
PHP: 7.1.24
Gantry5 installed: 5.4.24
Gantry5 upgrading to: 5.4.28

I have the problem, too, but I know that one of your assumptions is incorrect and I will get to that.

If anything is going to change about the code, this should be the workflow. It should continue to be full of sanity checks, but potentially gets around whatever issue you might be running into. At the end of the day, the API MUST ensure it can actually work with the file given otherwise everyone's better off just hardcoding the use of unlink() into their code.

Agreed.

* Check file actually exists, if not then return true (because it should never be considered an error to attempt to delete a non-existing filesystem resource, you wanted it gone and it's not there)

Agreed

* Check file is writable, if not then execute `chmod($file, 0777);` and fail if `chmod()` cannot be performed

I am not sure how you do the test whether the file is write able, but in MY case I am 100% sure that the file is write able by the user running the server. However, CHMOD is the problem, let me explain.

Assume "Apache" is the user running the webserver, for the sake of this example I named it the same as the type of server I use (Apache). Editor is the user allowed to edit and make changes and not, it's NOT root. Also "OTHER" is NOT allowed to see the files, this is the correct way to do this for many reasons (goes beyond the scope of what we want to do here).

 -rw-rw---- Editor Apache 3601 Aug 10 2018 EventListener.php

then the Apache Server ALLOWED to do delete, edit and move the file BUT the user of the server is NOT ALLOWED to do a FULL chmod as this goes fully against the LINUX/UNIX security philosophy.
In the example below I have su'ed into a shell AS the user running the server (Apache).
I have then echo'ed (well its an append) something to the file, then displayed whether it was appened.
At the end I did a CHMOD, this WILL and MUST fail on ANY UNIX/LINUX - and it should IMHO.

[Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>ls -al
total 56
drwxrwx---. 3 Editor Apache 4096 Mar 28 11:54 .
drwxrwx---. 6 Editor Apache 4096 Aug 10  2018 ..
drwxrwx---. 4 Editor Apache 4096 Aug 10  2018 Controller
-rw-rw----. 1 Editor Apache 7847 Mar 28 11:54 EventListener.php
-rw-rw----. 1 Editor Apache 3601 Aug 10  2018 Page.php
-rw-rw----. 1 Editor Apache 4839 Aug 10  2018 Particles.php
-rw-rw----. 1 Editor Apache 5170 Aug 10  2018 Router.php
-rw-rw----. 1 Editor Apache 3487 Aug 10  2018 Styles.php
-rw-rw----. 1 Editor Apache 4479 Aug 10  2018 ThemeList.php
-rw-rw----. 1 Editor Apache 3952 Aug 10  2018 Theme.php
[Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>echo "// a comment " > EventListener.php
[Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>tail -n 4 EventListener.php
        CacheHelper::cleanMenu();
    }
}
// a comment
[Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>chmod 0777 EventListener.php
chmod: changing permissions of `EventListener.php': Operation not permitted
[Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>

So you can see Apache is able to write to the file but cannot CHMOD.
Apache should not be able to change the file to 0777, if it could it would be ludicrous.

Change your check to "touch" the file or "move" the file to a temp file then reverse it but do NOT use CHMOD rather tell the user the operation was not successful.

Also I am not sure how you test whether the file is write able, clearly in my case it's 100% write able.
You do not need full 0666 permissions, this would allow OTHERS to see what's in the files and in many cases files contain information that should not be seen by the public.

I know many people say you should have 0664 on files, that is not needed. I am not the only system administrator that will tell you exactly this. The user running the server needs to be able to write and this is 100% sufficient:

 -rw-rw---- Editor Apache 3601 Aug 10 2018 EventListener.php
avatar jobst
jobst - comment - 28 Mar 2019

closed as Issue seems resolved. If not please reopen.

If I knew how I would ;-)

avatar orlitzky
orlitzky - comment - 6 Apr 2019

This also completely breaks every Joomla installation that uses filesystem ACLs to grant read/write access. Using "can I chmod" as a test is fucking stupid, and has no basis in anything at all for being correlated with "can I delete this file."

Here's the patch to fix it:

- if (!Path::canChmod($file))
+ if (!Path::canChmod($file) && false)
avatar HLeithner HLeithner - reopen - 6 Apr 2019
avatar HLeithner HLeithner - change - 6 Apr 2019
Status Closed New
Closed_Date 2019-03-04 12:16:52
Closed_By joomla-cms-bot
avatar HLeithner HLeithner - reopen - 6 Apr 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 6 Apr 2019
Labels Added: J3 Issue
avatar franz-wohlkoenig franz-wohlkoenig - labeled - 6 Apr 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 6 Apr 2019
Status New Discussion
avatar jobst
jobst - comment - 6 Apr 2019

This also completely breaks every Joomla installation that uses filesystem ACLs to grant read/write access. Using "can I chmod" as a test is fucking stupid, and has no basis in anything at all for being correlated with "can I delete this file."

I did not want to use "fucking stupid" in my explanation, but I totally agree!
It is as stupid as people saying you need to chmod every file to 0644 - which makes it WOLRD readable.
If you have different editors for virtual hosts they all can see each others files which in many cases is a security risk!

avatar JimDee
JimDee - comment - 13 Apr 2019

Just leaving a comment in case anyone comes here with this issue on a LiquidWeb CloudSites platform. They currently use their own permissions on things that aren't the standards recommended by J! devs. From time to time on that platform, you'll run into this issue and others that reek of weird permissions. The fix, in this case, is to simply get on w/ their tech support (as it's a managed platform) and say "hey, can you update the permissions on my domain". They get enough requests these days that they know what's needed & run a command or two... then all works well (including the fix for this issue, on this particular platform).


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23504.

avatar HLeithner
HLeithner - comment - 14 Apr 2019

So I think we need a solution and failing because we can't change the permissions makes no sense to me. I'm also not sure what happens if the filesystem has extended permissions.

avatar mbabker
mbabker - comment - 14 Apr 2019

Before making the assertion that "changing permissions should not cause delete to fail", you need to understand filesystem permissions. On a Linux filesystem, a delete action requires write permissions. So a file in a readonly state (400 or 444) cannot be deleted, you must execute chmod +w /path/to/file to get it into a write state (600, 644, or 664 depending on your user and group permissions) before you can delete the file. There is also a comment in the File::delete() method that notes of a Windows concern:

// Try making the file writable first. If it's read-only, it can't be deleted
// on Windows, even if the parent folder is writable

// Try making the file writable first. If it's read-only, it can't be deleted
// on Windows, even if the parent folder is writable

This concern needs to be validated and either confirmed to still be an issue on whatever Windows operating systems Joomla supports (there isn't a declaration of such anywhere, but there is a stated support of IIS 7.0 which was shipped with Windows Vista and Windows Server 2008, which to me implies those are the oldest Windows OS builds that need to be confirmed at a minimum for this problem) or if it is confirmed to no longer be an issue the comment removed from the code as it is misleading.

I laid out 3 months ago exactly what steps can be taken to lessen the impact of whatever problem people are running into in #23504 (comment) but it seems people would rather drop F-bombs and call those contributing to Joomla stupid than actually work toward a reasonable solution.

But, I'm in a particularly good mood today, so here's my last useful contribution to this issue, you all have fun deciding what to do with it. For S&G, I changed the delete method to this:

public static function delete($file)
{
	$files = (array) $file;

	foreach ($files as $file)
	{
		$file     = Path::clean($file);
		$filename = basename($file);

		// If the file does not exist, there is no need to execute the rest of this code
		if (!file_exists($file))
		{
			continue;
		}

		// Try to delete, if we succeed then skip going into the error handling code below
		if (@unlink($file))
		{
			continue;
		}

		$unlinkError        = error_get_last();
		$unlinkErrorMessage = !empty($unlinkError['message']) ? $unlinkError['message'] : 'Unknown Error';

		// If the file is already writable, bail out now
		if (strpos(Path::getPermissions($file), 'w') !== false)
		{
			throw new FilesystemException(
				sprintf('%s: Failed deleting %s with error "%s"', __METHOD__, $filename, $unlinkErrorMessage)
			);
		}

		// We're going to try and make the file writable to delete it, first make sure we can do this
		if (!Path::canChmod($file))
		{
			throw new FilesystemException(
				sprintf('%s: Failed deleting %s with error "%s"', __METHOD__, $filename, $unlinkErrorMessage)
			);
		}

		/*
		 * Try making the file writable first. If it's read-only, it can't be deleted on Windows,
		 * even if the parent folder is writable
		 */
		if (!Path::setPermissions($file, '0777'))
		{
			throw new FilesystemException(
				sprintf('%s: Failed deleting unwritable file %s', __METHOD__, $filename)
			);
		}

		// One last try to delete
		if (!@unlink($file))
		{
			$unlinkError        = error_get_last();
			$unlinkErrorMessage = !empty($unlinkError['message']) ? $unlinkError['message'] : 'Unknown Error';

			throw new FilesystemException(
				sprintf('%s: Failed deleting %s with error "%s"', __METHOD__, $filename, $unlinkErrorMessage)
			);
		}
	}

	return true;
}

With this added test case:

public function testDeleteForReadOnlyFile()
{
	$this->skipIfUnableToChmod();

	$name = 'tempFile';
	$data = 'Lorem ipsum dolor sit amet';

	if (!File::write($this->testPath . '/' . $name, $data))
	{
		$this->markTestSkipped('The test file could not be created.');
	}

	$this->assertTrue(
		Path::setPermissions($this->testPath . '/' . $name, '0400')
	);

	$this->assertTrue(
		File::delete($this->testPath . '/' . $name),
		'The file was not deleted.'
	);
}

None of the error handling code ever actually gets triggered. I then changed the setPermissions call to Path::setPermissions($this->testPath, '0400', '0511') to put the folder and all of its files into a readonly mode. Then that test case started failing. Dumping the result of Path::getPermissions($this->testPath . '/' . $name) after the unlink() call, I found that file had already been chmod to 777 by PHP itself (PHP 7.2.10, Homebrew PHP install on MacOS 10.14.3; I only have PHP 7.1 and 7.2 at my disposal so don't go looking for PHP 5.3 results from me) and yet I continued to receive a unlink(/private/var/folders/b1/db5lfb3s37d1qbwy17r6s_y40000gn/T/1555256492.1502.463511382/tempFile): Permission denied error message. So even with the file writable, the delete operation failed (as expected) because the folder is read-only.

So, if someone wants to confirm behaviors across operating systems and PHP versions, it needs to be determined how PHP is behaving and a plan formulated on how to change things, probably in CMS 4.0 and Framework 2.0, accordingly. I would seriously suggest making liberal use of error_get_last() in the filesystem API and expanding the Exception subclass it throws to include the PHP error message as a separate property (similar to query errors, otherwise you're easily creating a path disclosure problem because the CMS doesn't do adequate error handling) no matter what though because the fact that the filesystem API absorbs errors and doesn't do anything useful to help people debug problems is quite sad.

Good luck.

avatar brianteeman
brianteeman - comment - 14 Apr 2019

on how to change things, probably in CMS 4.

If that is the case then we only need to be concerned with php 7+

avatar HLeithner
HLeithner - comment - 14 Apr 2019

thx @mbabker that function is looks much better, at least as concept. I'm not sure who far we should go trying to delete a file. As you mention under posix file system (confirmed for linux) its not possible to delete a file if the folder is write only (even if you are the owner). So in this case we have to change the write permissions for the folder.

if we do the magic for a windows version we should do the same for posix filesystem folder permissions.

btw. I would set the poermissions back to the original after deleting a file successful (folder) and before we thru an exception.

avatar jobst
jobst - comment - 14 Apr 2019

@mbaker the last suggestion will not work on a unix based system, you cannot chmod 0777, it will refuse to do this - as it should it goes 100% against every security principles of unix.

Below in he code section the file EventListener.php is RW (read and write) for the user running the webserver (Apache). It still fails on my servers upgrading. I have never ever had any problems before with any Joomla installation for any website on any of my servers until the last upgrade, now it is broken.

To show the problem I logged into the server as the user Apache (user running the server) and did a CHMOD 0777 on THAT file and you can see it it does NOT work (neither should it work as it would be an immense security issue).

First you see I appended some stuff to the file which shows the file allows write/append, then I did a CHMOD on that file which failed - you see it is write able but you should not be able to change the permission TO WORLD can read/write.

`
[Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>ls -al
total 56
drwxrwx---. 3 Editor Apache 4096 Mar 28 11:54 .
drwxrwx---. 6 Editor Apache 4096 Aug 10 2018 ..
drwxrwx---. 4 Editor Apache 4096 Aug 10 2018 Controller
-rw-rw----. 1 Editor Apache 7847 Mar 28 11:54 EventListener.php
-rw-rw----. 1 Editor Apache 3601 Aug 10 2018 Page.php
-rw-rw----. 1 Editor Apache 4839 Aug 10 2018 Particles.php
-rw-rw----. 1 Editor Apache 5170 Aug 10 2018 Router.php
-rw-rw----. 1 Editor Apache 3487 Aug 10 2018 Styles.php
-rw-rw----. 1 Editor Apache 4479 Aug 10 2018 ThemeList.php
-rw-rw----. 1 Editor Apache 3952 Aug 10 2018 Theme.php

append a comment at the end of the file and then show the last four lines

[Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>echo "// a comment " > EventListener.php
[Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>tail -n 4 EventListener.php
CacheHelper::cleanMenu();
}
}
// a comment

CHMOD the file to EVERYONE can read write

[Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>chmod 0777 EventListener.php
chmod: changing permissions of EventListener.php': Operation not permitted [Apache@MACHINE /WebsiteRoot/libraries/gantry5/classes/Gantry/Admin] $>

You cannot do something the operating system simply does not allow - a user cannot change a file to be world read/write, this would be a security flaw in itself.

Also there might be a much better approach - in case you are worried not being able to re-do something. Before you do the REAL copy/delete you go through the same file tree and CHECK whether EVERY file that needs to be writeable IS writeable, if NOT stop the process.
Call it PRE-CHECK - if it fails tell the user which file if stuffed, this way you do not need to reverse.
So when you do the real run later, you will NOT fail.

But just do not use CHMOD, you cannot tell UNIX what you want it to do, so work around it.

avatar mbabker
mbabker - comment - 14 Apr 2019

Actually, yes, you can chmod a resource to 777 if you have the permissions to do so. Additionally, with many PHP filesystem operations, the umask (default 0022) is used and that converts a 0777 permission to 0755 for directories and 0644 for files (default config normally, it's actually a little harder to get PHP to set something to 777 than you may realize). And, as I mentioned before, in the scenario where the directory is readonly, the call to unlink() resulted in PHP setting the file to 0777 and failing to delete it because the folder is not writable. So no, chmod($file, 0777); in and of itself is not this super magical security flaw.

Below in he code section the file EventListener.php is RW (read and write) for the user running the webserver (Apache). It still fails on my servers upgrading. I have never ever had any problems before with any Joomla installation for any website on any of my servers until the last upgrade, now it is broken.

Please provide a reproduction scenario that can be created against the core Joomla API. The use of Gantry in your scenarios does not provide a valid means for core to add test cases to its unit testing library to prevent regressions, and I am not going to spend the minimal time I do feel interested in looking at GitHub digging into third party libraries.

Again, I'm done with this issue. I have provided all of the insights that I can, I have provided all of the potential courses of action that can address whatever problem people are running into, even without the ability to create the problem myself. It's on the rest of the "core team" to do something now because I just don't care enough to go any further here.

avatar mahagr
mahagr - comment - 15 Apr 2019

Just to clarify: having 0777 in any other method than chmod() is totally safe because of umask() which is being applied to the permissions (read the PHP documentation!). And yes, chmod(0777) (which isn't using the umask, so the file will actually be using 0777) before delete is also safe because of only the user with proper permissions to change the settings can delete the file. It just ensures that the file can be deleted in case if something prevented writes on that file.

PS: The operation from the command line isn't permitted as your user does not have permissions to do the chmod action at all. Try using 0755 (or 0750) and it will fail too. You need to change your user to be able to change those permissions. Linux does not prevent you from changing your own files to world writable, you can try it and it'll just work.

avatar jobst
jobst - comment - 15 Apr 2019

@mahagr let me explain again.

I have never ever had problems with upgrades until the last update, it always worked on all of my Joomla websites. I also know security back and forth and how to apply it and that chmod will fail in many cases as you already see as I am NOT the only one complaining.

If you want to run webserver securely making sure you limit hacking in every possible way you can (its never 100% but close) one of the BIG security things: you do not have the USER running the web server be the OWNER of the tree - this is a no no, the OWNER of the tree is the EDITOR, the user running the web server is the GROUP.

Say you have a large of virtual hosts, each host has it's own editor, e.g.
drwxr-x---. 3 /var/www/website1/htdocs/ Editor1 Apache drwxr-x---. 3 /var/www/website2/htdocs/ Editor2 Apache drwxr-x---. 3 /var/www/website3/htdocs/ Editor3 Apache
You prevent Editor1 have access to website2 and website3 in case there are very important information in that tree. Apache (the user running the webserver) has access

By the way one of my favorite CRM's have learned this exact lesson, they PRE-CHECK every file they need to write before they actually do the UPGRADE and then tell the user "cannot write this file", then you have to fix it and restart the PRE-CHECK until everything is fine - ONLY then they do the real thing. For years they had that exact the problem they could not reverse the changes made because of failures.

PS: You do not seem to understand Unix very well, it will not work, CHMOD IS THE PROBLEM!
There is NO other way in UNIX to give permission to change the file other than put it into sudoers.
But that EXACTLY defeats the purpose of keeping a web server safe.

avatar HLeithner
HLeithner - comment - 15 Apr 2019

The only reason why we chmod is a IIS 7 issue, for normal POSIX systems this has no effect because if we can change permissions on a file doesn't not mean that we can delete it.

At least in theory see #23228

Maybe @jobst and @mahagr want to understand that we also support other systems then linux. Michael showed a solution that works, setting it to 777 makes no sense on linux but as the comment says it seams to be needed for windows.

avatar jobst
jobst - comment - 15 Apr 2019

@HLeithner never said I do not understand you are supporting other systems, that is totally fine and all part of being a CMS ... it's just chmod has problems on Linux and that's all I am pointing out!

avatar mahagr
mahagr - comment - 15 Apr 2019

I've been using Linux as my main operating system since 1996 and for 7 years my main job was programming in Linux kernel space in a security company (Firewall/IPS), so I do know POSIX pretty well. I also used to be a security expert and I do know the dangers of using world-writable permissions. I'm not saying that using 0777 is not bad, I'm saying that the code isn't doing that (with the exception of deleting files).

But on the contrary, I can say that using 0755 inside PHP code is BAD, because of it will prevent setups like yours from working because of Apache group isn't allowed to modify or delete the files. Using 0777 (with umask!) works and is safe in every single properly configured setup, 0755 does not because of it will not allow Apache to modify the files. And this is true primarily in Linux.

EDIT: Again, the only place where you should not use 0777 in PHP is chmod() command, which does not use umask. In there you should write chmod(0777 ~ umask()) for folders and without x for files.

avatar mahagr
mahagr - comment - 15 Apr 2019

Just something I noticed from this:

drwxr-x---. 3 /var/www/website1/htdocs/ Editor1 Apache

Unless you are running your web server as Editor1, Joomla is not allowed to modify any of the files in your site. This means that you need to do Joomla updates manually by copying the files by hand and running manually the upgrade scripts.

Another note on the upgrades starting to suddenly fail: there was a bug in Joomla where read-only files were silently skipped by the installer -- installation process passed, but some files were left behind without an update. While not optimal, now the installer at least fails if some of the files cannot be changed, which is a great improvement in my eyes and saves a lot of time for everyone when things do not work after the upgrade. Not that it cannot be improved, I do agree that pre-check before installation may be a good idea.

avatar ReLater
ReLater - comment - 15 Apr 2019

@ mbabker has provided some code in #23504 (comment) .

Now my question to the experts here is: Why don't you provide "better" code with detailed and reproducable testing instructions and code examples that apply to Joomla core?

Anybody can test it then.

avatar orlitzky
orlitzky - comment - 15 Apr 2019

Before making the assertion that "changing permissions should not cause delete to fail", you need to understand filesystem permissions. On a Linux filesystem, a delete action requires write permissions. So a file in a readonly state (400 or 444) cannot be deleted

No:

$ mkdir permissions-test
$ touch permissions-test/example.txt
$ sudo chown root:root permissions-test/example.txt
$ sudo chmod 400 permissions-test/example.txt
$ rm -f permissions-test/example.txt
avatar jobst
jobst - comment - 15 Apr 2019

Just something I noticed from this:

drwxr-x---. 3 /var/www/website1/htdocs/ Editor1 Apache

My bad, I used this as an example.
I made this up (my tree is somewhere else) and forgot to change the group write.
Sorry.

avatar miojamo
miojamo - comment - 6 Jan 2020

This also completely breaks every Joomla installation that uses filesystem ACLs to grant read/write access. Using "can I chmod" as a test is fucking stupid, and has no basis in anything at all for being correlated with "can I delete this file."

Here's the patch to fix it:

- if (!Path::canChmod($file))
+ if (!Path::canChmod($file) && false)

I have this issue under Windows Ubuntu 18.04 WSL, mounted directory with all permissions set to 777 by default by WSL. This patch worked great! There is no other solution so far. Thanks.

avatar kris-sum
kris-sum - comment - 31 Jan 2020

I have this issue under Windows Ubuntu 18.04 WSL, mounted directory with all permissions set to 777 by default by WSL. This patch worked great! There is no other solution so far. Thanks.

^ THIS!
It seems that depending on how your WSL filesystem is mounted, the chmod will fail. Been scratching my head over this for a couple of hours!

avatar miojamo
miojamo - comment - 31 Jan 2020

@kris-sum Do you have any other solution for the problem? I've tried to reinstall Ubuntu, but issue persist
Also asked here with no answer:
https://askubuntu.com/questions/1203423/windows-10-and-ubuntu-18-04-wsl-folders-permissions-problem

avatar billtomczak
billtomczak - comment - 31 Jan 2020

Just an FYI on this issue in case it might be helpful to others still struggling with this issue

We have been using Cloudways servers for our staging and demo sites and can now reliably reproduce this problem. The way they've organized their system, if you use the 'Reset Permissions' option for a site on a server, ownership is given to the master/root user of the server. The only fix is to call their support team and have them set ownership back to the site user. They are aware of the issue and claim they will fix it someday.

Whenever, for whatever reason, the right (wrong?) file(s) are owned by the master user, we'll start having this problem. A quick call to Cloudways support to fix all the file/directory ownership for a site solves the problem.

avatar orlitzky
orlitzky - comment - 31 Jan 2020

Just patch out the problematic line. It's wrong and never should have been added. This isn't a problem with your systems, it's a problem with joomla.

avatar jwaisner jwaisner - change - 17 Apr 2020
Status Discussion Confirmed
avatar OctavianC
OctavianC - comment - 30 Apr 2020

Just ran into this exact issue on a customer's server. $file is 0777 and can be deleted, but canChmod() fails for some reason and this prevents the installation from continuing. Simply commenting that line out resolves the issue.

avatar mbabker
mbabker - comment - 30 Apr 2020

I find it funny I wrote proof of concept code a year ago that could have theoretically addressed this issue, and yet here we are, a year later, issue still open, and people still instructing to core hack to bypass the issue.

Can someone else please provide some code? Maybe someone will act on it if it didn't come from me. That seems to have worked a few times in regards to getting pull requests actioned.

avatar orlitzky
orlitzky - comment - 30 Apr 2020

Your proof-of-concept doesn't work because there's a bug in your understanding of UNIX permissions: "On a Linux filesystem, a delete action requires write permissions." That's clearly false, as #23504 (comment) shows. Checking for write permissions (or the ability to chmod, or anything else other than the ability to actually delete the thing) is therefore pointless and counterproductive in those cases.

I've already proposed working code: delete that line. That is the fix. You don't need to use heuristics to guess if you can delete a file. You try it, and if you can't, it fails and you handle the error.

avatar mbabker
mbabker - comment - 1 May 2020

You're actually right this time, read-write is not necessary on the file in question. It is necessary on the directory which owns the file. Therefore, while removing the check at the file level would be a valid fix, Joomla as an end user facing software should still attempt to check and potentially reset filesystem permissions on the owning directory. Or, do a permissions check before performing the delete operation and fail with a more appropriate error message in the case that the prerequisite permissions are not in place as necessary.

Or, just do what you suggested, blindly remove some code from the platform without actually identifying the root cause of the issue. That always seems to go over well.

You are welcome to make your change proposal as a pull request. Actually, anyone in this thread is. Or, y'all can keep bitching while waiting for an unnamed someone to do something, because I sure as shit ain't doing it.

avatar mbabker
mbabker - comment - 1 May 2020

BTW, the @chmod($file, 0777); line has existed (with the note regarding Windows) since c02dfed from July 2007. It has operated without any form of error check, and has been explicitly error suppressed since July 2007. I don't know how you deliver code or the types of QA measures that you use, but to me error suppression is a red flag that something is failing to do adequate checks or raise appropriate errors in the right conditions. So yeah, a Path::canChmod() check before blindly calling chmod() makes sense. Unless you just say "screw error handling, it's for suckers".

avatar orlitzky
orlitzky - comment - 1 May 2020

You're actually right this time, read-write is not necessary on the file in question. It is necessary on the directory which owns the file.

Also incorrect, since write-only permissions suffice on any parent directory of that directory. Or write permissions can be granted with POSIX filesystem ACLs, which change the meaning of the w bit. Or explicit "delete" permissions can be given with the NFSv4 ACLs on OSX. Or god knows what can happen on Windows these days.

Therefore, while removing the check at the file level would be a valid fix, Joomla as an end user facing software should still attempt to check and potentially reset filesystem permissions on the owning directory.

I have no idea what you think we should be resetting the permissions to, but this is doomed to fail since you can't even tell me what sort of access controls are in use on the parent directory.

Or, do a permissions check before performing the delete operation and fail with a more appropriate error message in the case that the prerequisite permissions are not in place as necessary.

Even if you could write a working permissions check (you can't), the error message is always going to say roughly the same thing: delete failed for some reason. PHP doesn't provide a better API than that. The difference is that, with an incorrect permissions check, you also throw that error when there is no problem (hence, this bug report).

Or, just do what you suggested, blindly remove some code from the platform without actually identifying the root cause of the issue. That always seems to go over well.

It's hardly blind. The root cause has been identified: someone thought they knew how to tell if a file could be deleted, and they were wrong. Now we're all getting errors when our files are deletable but the buggy check fails. The solution is to remove the buggy check, and to raise/log an error if deleting the file actually fails. What we have now is complicated and doesn't work. Deleting the check is simple and does work. It shouldn't be this hard to sell.

avatar mbabker
mbabker - comment - 1 May 2020

Then the chmod() action needs to go too. Which means validating whether the issue it was working around for Windows based systems needs to be validated. If it needs to stay, then a split path for Windows versus Linux should be implemented if attempting chmod() is really that much of a problem.

Seriously, using error suppression to perform an action, then not even validate that the action succeeds or fails, is about as bad as having a buggy check before attempting said action. Actually, I'd call it borderline worse, because the developer implementing it knew damn well there could be error conditions coming out of the call but was too lazy to do anything with them.

avatar orlitzky
orlitzky - comment - 1 May 2020

Then the chmod() action needs to go too. Which means validating whether the issue it was working around for Windows based systems needs to be validated. If it needs to stay, then a split path for Windows versus Linux should be implemented if attempting chmod() is really that much of a problem.

No argument there. Personally, I would say that if someone sets the "read-only" flag on a file in Windows, we should do the obvious thing and respect it rather than toggle it off to subsequently be bypassed. Using the read-only flag for access control is the wrong solution to begin with in that situation, as evidenced by the fact that it can be bypassed at all. In other words, the best solution to the problem was probably to ask the user "why did you do that?" way back before the chmod line was added to this function.

Seriously, using error suppression to perform an action, then not even validate that the action succeeds or fails, is about as bad as having a buggy check before attempting said action. Actually, I'd call it borderline worse, because the developer implementing it knew damn well there could be error conditions coming out of the call but was too lazy to do anything with them.

The error isn't suppressed if you skip the canChmod check, it just raises a less descriptive but more accurate error if/when the unlink fails:

if (!@ unlink($file))
{
        throw new FilesystemException(__METHOD__ . ': Failed deleting ' . $filename);
}

Whether or not that's a good idea is debatable, given how crude the API is. If the unlink fails because the file was already deleted, is that an error? But let's not stray too far from the original topic.

Add a Comment

Login with GitHub to post a comment