The drop is always movingYou know that saying about standing on the shoulders of giants? Drupal is standing on a huge pile of midgetsAll content management systems suck, Drupal just happens to suck less.Popular open source software is more secure than unpopular open source software, because insecure software becomes unpopular fast. [That doesn't happen for proprietary software.]Drupal makes sandwiches happen.There is a module for that

Writing PHP files from Drupal

Submitted by nk on Mon, 2012-09-03 14:50

Over the years the security teams consistently shut down projects that were updating modules from Drupal. In Drupal 7, we added a subsystem where you could FTP or SSH back to the server to do that -- and allowed for local writes in insecure setups. In Drupal 8, however, we now have a feature in core that allows us to write PHP files just like that. I feel the community deserves an explanation of why we didn't allow these modules in the past and what is different in the D8 API. The problem is that if your web server setup is such that Apache can write PHP files into directories it serves PHP files from then a poorly written upload script like this one can allow an attacker to upload a PHP file and execute it (or, upload a .module and get Drupal to include and execute it). There are a healthy number of protections in the D8 API:

  1. The PHP files are written into the files directory which has .htaccess protection against executing PHP files.
  2. The name of the files are constructed in a way that an attacker can not reproduce: it depends on the modification time of the directory it is in and a secret value.
  3. If Drupal includes a PHP file which generates an error and thus reveals the file name, an attacker still can't use that file name for an upload. (If the file is deleted and written back then the directory mtime changes, if the file is just opened for writing then its own mtime will be higher than the directory which Drupal refuses to include.)
  4. The directory where the file is not neither writeable nor listable.
  5. The file is not writeable either.

Basically, our hope is that adding this new API does not open new avenues for an attack. For example, if you have local filesystem access then while you could exploit this new subsystem, you also could do other things (like reading settings.php) so then this subsystem does not open new doors for attack.

There is a small new risk. If all the following are true:

  1. Error display is set to screen (should not be such on production).
  2. The PHP code written to disk shows an error revealing the file name.
  3. There is such a "leaky" upload script.

then the attacker has an incredibly short window (a few PHP instructions long) to replace the newly written PHP file with an attack script. If the upload script also is broken enough to chmod the holding directory then this window becomes one second long.

The immediate plan is to use this for the dependency injection container. In the future, Twig and perhaps module upgrades could use it. The code is self contained enough that we could even use it upgrade core.

Commenting on this Story is closed.

Submitted by Anonymous on Tue, 2012-09-04 02:30.

Sounds like a good idea in theory. I think it would be worth getting some penetration testers to check out how that part of the system could be exploited to try to build some extra protections into it.

Submitted by cam8001 on Tue, 2012-09-04 09:28.

Can you link to the issues where this functionality was added? Is this to do with compiling the Dependency Injection Container?

Submitted by Anonymous on Tue, 2012-09-04 12:44.

Seems to be this one:

Submitted by Anonymous on Wed, 2012-10-03 07:46.

Often it is difficult to take a decision and as a rule, in such a situation, people look for someone’s advice. At the same time it is quite difficult to say whether their final decision is that of their own or it is the result of the piece of advice they have got.

Submitted by Anonymous on Tue, 2012-09-04 11:58.

Why not just take a cryptographic signature of the file and store it along side that file instead of all of this?

Have a setting that has a unique and random cryptographic key in the settings file.

When generating a new file, compute a HMAC of the file using the cryptographic key, and save it in another file (prefix the name with a "." for instances).

Then, when loading, just verify that the signature matches. If so, you know the file couldn't have gotten there by someone who doesn't have your key.

The benefit here is that it doesn't rely on OS behaviors (mtime) and it's a lot simpler.

This is basically what a signed phar does.

In fact, why not generate those files into a phar, and sign it. That way there's a single repository of generated files. You could use a stream wrapper to write to the phar (which shouldn't be often at all), and then a stream wrapper to "include" files from it. Then you'd only have to worry about signing a single file...

Submitted by nk on Tue, 2012-09-04 19:58.

One word: speed. HMACs against the whole file (instead of the file name) are expensive to verify on every read. And phar generation is not working because that can be disabled on a host level and then Drupal won't work on those hosts and there's nothing we can do about.