get_filesystem_method() uses uses wrong owner for validation

[cyberspice]

The function get_filesystem_method() creates a temporary file and then checks the ownership comparing it with the result of getmyuid(). getmyuid() does not return the owner of the web server process but of the file calling getmyid(). In this case wp-admin/includes/file.php. This means that in order to support automatic update the Wordpress files have to be own by the same process as the webserver regardless of permissions on the files. This is a potential security risk.

If posix_getiud() is used where available then the owner of the webserver process is compared to the ownership of the temp file. The Wordpress files can be owned by someone else and the update system works as long as the webserver has permissions to write.

I have written more about this (together with a fix) in my bog.

​http://www.cyberspice.org.uk/blog/2009/07/15/wordpress-automatic-update-bug/

[dd32]

This is a potential security risk.

Can you explain that a bit further before i just close this as duplicate and refer you to the previous tickets/explanations?

The Wordpress files can be owned by someone else and the update system works as long as the webserver has permissions to write.

If that was the intended direction, I'd have just used an is_writable() call.

The intended aim of that code, Is that if the owner of the Created files, Differs from that of the currently running process then to NOT use the direct setup, due to the created files being owned by someone other than the actual user.

Another way that code could be written which may work on more hosts is:

if ( file_owner($temp_file) !== false && file_owner(__FILE__) === file_owner($temp_file) ) {
//use direct
}

but that would NOT cause the direct setup to be used in your situation either..

the usage of getmyuid() was used to replace fileowner(__FILE__) basically.

Still tempted to close as duplicate, but might be worth changing the code, or at least adding a comment that its the intended code..

[dd32]

see ​http://core.trac.wordpress.org/search?q=get_filesystem_method for a tonne of previous tickets.

[Denis-de-Bernardy]

#10205

[Denis-de-Bernardy]

maybe we could add a filter, though. you know... so that users who want to check this differently can do so.

[dd32]

maybe we could add a filter, though. you know... so that users who want to check this differently can do so

Then they can use the filter already present, or force it through the constant. Please stop closing things so soon.

Re-opening pending close due to a commit to add some documentation or coding it differently to show that its not a typo.

[dd32]

ok, i'll make a new ticket for that instead..