[theme-reviewers] Theme submission fails on WARNING

[Otto]

On Fri, Jun 24, 2011 at 4:14 PM, Bruce Wampler <brucewampler at gmail.com> wrote:
> To my shock, the latest version of the theme was rejected by the standard
> upload process with only one WARNING. In the past, WARNINGS were just that -
> something that need further review before approval. Now apparently, WARNINGS
> cause automatic rejection.

Yes, we decided to turn the screw a bit harder a few months ago. The
major elements of the theme check is now a required pass for
uploading.

> In this case, the only WARNING issued by the submission process for fopen.
> ...
> There are no security issues with the way Weaver uses file
> operations.
> ...
> have you really banned fopen?

Yes, fopen, and other direct file operations, are banned. Writing
files from the theme (or from a plugin) in the most direct manner is
inherently insecure on quite a number of shared hosting environments.

I go into this in more detail here:
http://ottopress.com/2011/tutorial-using-the-wp_filesystem/ but I'll
give you the quick version.

As you're a PhD and a software developer, I'll assume you understand
UNIX file ownership and process permissions and such. In your typical
shared hosting environment, several users all have directories for
their website files. These files are owned by the user account. The
web process (let's say apache), runs as a different, unprivileged user
(lets say "nobody").

When the PHP code is executing, it's executing with the credentials of
that nobody user. So when it makes files, the resulting files are
owned by "nobody".

Here's the problem: If the files are owned by the apache process, then
anybody else who can get that apache process to execute their code can
write to those files, by simply writing code that writes to them then
running it on their own websites. Since we're in a shared hosting
environment, everybody else on that web server now has the ability, by
writing code on their own websites, to modify the files of somebody
else's website on the same server. This works irrespective of normal
600 type permissions, because the process writing to the files
(nobody) is the same one that owns the files (nobody).

Writing files as nobody also introduces problems on some hosts where
the user themselves find they can't delete or modify files in their
own directory without doing it through the webserver process.

And if other users can write files that are used on your website, even
if they're only CSS files, then that's a rather obvious security
problem. With just CSS I can still inject javascript code into your
site and steal your credentials and so on.

The solution is that files should be owned by the user's own account,
not by the apache process.

> Do you really want to take away advanced
> features that require it from WordPress users?

Not at all. There are alternative methods, although they have drawbacks.

When WordPress self-upgrades, it goes to pains to make sure that not
only can it create and write files, but that the resulting files are
owned by the correct user account and not by some other user account.
It uses one of five or so methods to write files, depending on the
server configuration. This is all packaged up into the WP_Filesystem
classes and there are methods for using it which I detailed on my
blog: http://ottopress.com/2011/tutorial-using-the-wp_filesystem/

The drawback is that whenever you have to write files, you sometimes
(depending on the server configuration) have to ask the user for their
FTP credentials, so that it can write files via that loopback method,
thus creating files as the user instead of creating them as the
webserver process. If the servers are configured using the setuid
method (where the PHP process changes its user to be the user of the
owner of the PHP files), then the "direct" method is used by the
WP_Filesystem, and no credentials are required. This is actually safer
in shared hosting, and becoming more and more common. Even cheap
GoDaddy hosting uses setuid methods.

This may require a rethinking of your file writing operations and/or
your interface, but in the end, it's worth it for security reasons.

-Otto