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.
Labels |
Added:
?
|
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.
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.
chmod($file, 0777);
and fail if chmod()
cannot be performedunlink()
, if this fails and chmod()
was executed attempt to restore to the previous stateI agree this is the proper way to do this.
Recently have similar issues with an extension update ( and un-install, too! ) see https://forum.joomla.org/viewtopic.php?f=728&t=968897
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.
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
You can't have a new bug in an API that had zero changes in 3 releases.
Well, some very file-system-like problems suddenly appear after the J3.9.2 update - what else could be causing these behaviors?
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.
@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.
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.
@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.
Safe mode was removed in php 5.4.0 so if that is the problem there is an obvious and easy answer
We manage over 100 joomla sites, we are on:
Php 7.0.7
We have found for SOME tools (not all)
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
@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.
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
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
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
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.
@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.
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-03-04 12:16:52 |
Closed_By | ⇒ | franz-wohlkoenig |
Closed_By | franz-wohlkoenig | ⇒ | joomla-cms-bot |
Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/23504
closed as Issue seems resolved. If not please reopen.
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
closed as Issue seems resolved. If not please reopen.
If I knew how I would ;-)
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)
Status | Closed | ⇒ | New |
Closed_Date | 2019-03-04 12:16:52 | ⇒ | |
Closed_By | joomla-cms-bot | ⇒ |
Labels |
Added:
J3 Issue
|
Status | New | ⇒ | Discussion |
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!
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).
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.
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:
joomla-cms/libraries/src/Filesystem/File.php
Lines 209 to 210 in a81ada4
joomla-cms/libraries/vendor/joomla/filesystem/src/File.php
Lines 129 to 130 in a81ada4
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.
on how to change things, probably in CMS 4.
If that is the case then we only need to be concerned with php 7+
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.
@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
[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] $>
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.
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.
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.
@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.
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.
@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!
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.
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.
@ 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.
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
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.
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.
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!
@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
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.
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.
Status | Discussion | ⇒ | Confirmed |
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.
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.
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.
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.
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".
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.
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.
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 attemptingchmod()
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.
@HLeithner I see that you assigned this to yourself. It looks to me that there have been no changes relating to this and it is still a valid issue for J4.
Can someone please update the label to J4 Issue and if @HLeithner says its not a valid issue in j4 then they can close it
Labels |
Added:
No Code Attached Yet
J4 Issue
Removed: J3 Issue ? |
Labels |
Added:
bug
|
-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 executingunlink()
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 causingchmod(0777)
to put the file into a read-only state.