WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#7779 closed defect (bug) (invalid)

Automatic plugin upgrade doesn't detect the effective uid correctly.

Reported by: jamuraa Owned by: DD32
Milestone: Priority: normal
Severity: normal Version: 2.6.1
Component: Administration Keywords: reporter-feedback
Focuses: Cc:

Description

When trying to upgrade a plugin automatically, the system attempts to detect if it can create files, and fails because it is using getmyuid() to try to find out the effective user ID. getmyuid() returns the owner of the file that is currently being executed, not the user ID executing it. This means if you are running php in a fcgi process (as root, nobody, or any other user other than the owner of index.php), it fails to recognize that it can actually create and delete files.

The patch below used posix_getuid() which returns the actual UID of the process running, and allows automatic upgrades to happen normally on my system.

Attachments (1)

getmyuidbad.patch (606 bytes) - added by jamuraa 6 years ago.
Patch to use posix_getuid() instead of getmyuid()

Download all attachments as: .zip

Change History (12)

jamuraa6 years ago

Patch to use posix_getuid() instead of getmyuid()

comment:1 jamuraa6 years ago

  • Summary changed from Automatic upgrade doesn't detect the effective uid correctly. to Automatic plugin upgrade doesn't detect the effective uid correctly.

comment:2 ryan6 years ago

  • Owner changed from anonymous to DD32

comment:3 DD326 years ago

After glancing at the PHP docs, the patch looks the way to go. I'll take a closer look and do some testing tomorrow.

comment:4 DD326 years ago

  • Keywords reporter-feedback added

Ok, After reading the docs and checking with some tests, No, This isnt the way to go.

getmyuid() returns the user ID of the current script
posix_getuid() returns the user ID of the current process

Are there any examples of a case where it doesnt detect the owner correctly?

A Better way of writing that function could be:

if ( fileowner(ABSPATH . '/wp-settings.php') == fileowner($temp_file) ) 

instead of

if ( getmyuid() == fileowner($temp_file) ) 

That way it's comparing file owners instead, and not relying on the current script knowing its effective UID -- But i dont have any setups i can test that on (They all work with getmyuid() ok)

(Note: I've only got 2 setups which i can test direct access on at present, definately not a cover-all-bases setup)

Setting to Reporter-Feedback for examples of a case where direct fs access is available, but not used.

comment:5 jamuraa6 years ago

I have a case like this, and I would expect that it is the case on many servers. The webserver runs as www-data, and the files are owned by jamuraa (myself). The webserver can write to every directory and file in the site directory because the files are set to group www-data, and the group write bits are set (and sgid as well). I am not running suPHP.

The file is created with uid www-data and group www-data, correctly because the process is running as www-data. The script is owned by jamuraa (getmyuid() returns jamuraa's uid).

This bug specifically occurs because the effective UID is not the same as the UID which owns the script in the filesystem, but the process has rights to write to all the files required.

To summarize:

webserver runs as user www-data 
files are owned by jamuraa
webserver runs the php script as user www-data

getmyuid() returns jamuraa
posix_getuid() returns www-data
$temp_file gets written with owner www-data

Applying the patch above allows direct fs access, if I switch back to getmyuid(), I get a ftp page.

If you need any more information just ask, I'm watching the bug.

comment:6 DD326 years ago

  • Resolution set to invalid
  • Status changed from new to closed
webserver runs as user www-data 
files are owned by jamuraa
webserver runs the php script as user www-data

getmyuid() returns jamuraa
posix_getuid() returns www-data
$temp_file gets written with owner www-data

Then the idea was, That you should not use the Direct FS access because files created by the web server are not owned by the same user that owns the WordPress files, It creates headaches when a user comes along and wants to delete a file via FTP, and finds that, because they're running as user xyz and not www-data, they cant delete files out of their own directory.

Now, Some Server setups are as such that users CAN delete files created by the web server(because the group bits are set correctly & the user is part of the group), but there are a number which disallow it as well.

If you want to force Direct FS access rather than FTP, You may do so by returning 'direct' on the filter filesystem_method I'll get around to releasing a plugin which can override all the FS defaults at some stage..

comment:7 joostdevalk5 years ago

I think this should somehow be fixed in core, but for now, I've written a plugin that does as DD32 suggested, which you can find here: http://yoast.com/fix-automatic-plugin-update/

comment:8 DD325 years ago

I think this should somehow be fixed in core

It cannot be fixed in core, As there is nothing to fix.

If the files are owned by a different than the webserver runs as, Then deleting/modifying/adding files will cause them to be owned by the webserver, not as the user, which leads to deletetion problems. There are other options, Such as Group-writable which would enable the direct method to be used by more people, It however, Would have the issue of handling much more complex code, due to having to change the othership of the file (Both for allowing access to the files for the user via ftp, And for the webserver owner (in the case of shared hosting) keeping track of users quota's -- I dont think WordPress would want to go against them..)

comment:9 DD325 years ago

There is already at least one person who's commented on your plugin of the "Cannot remove plugin" error, That ends up being caused directly because the webserver doesnt have the access to modify the users files, As it should be, Which is why WordPress chooses not to use that method. (Yes, I've commented on that post, its in the moderation queue). This is mainly to those who come across this trac ticket.

comment:10 hakre5 years ago

Let's Code a File-System Layer into Wordpress that handels ALL File-System-Based Operations.

This will abstract all those access problems for the application logic which would be helpfull for the wordpress core as well as themes and plugins. This makes even integrating Wordpress easier.

It can be fully provided with a testsuit and php5 ooo coded sothat it will have a propper robustness as needed for such a compontent.

Any feedback?

comment:11 DD325 years ago

Let's Code a File-System Layer into Wordpress that handels ALL File-System-Based Operations.

Problem there, Is that credentials are required, And most of us do not like storing our creds. on the server in plaintext (as WP would require). Also, The current layer does exactly that, Its just up to if anyone wants to utilise it for uploads & file editors.. uploads tend to be something people dont want to mess with, they just want it done, no cred. requests..

So, What do you see that needs to be done? More areas moved over to it? or what? Feel free to open a new ticket for this, as this ticket is for a different topic ( But make sure to explain the differences between what there currently is, and what you envision )

Note: See TracTickets for help on using tickets.