Composer security issues

[gamort]

The current documentation/implementation of composer allows for a large number of attack surfaces.

A "deployed" website[ie a website either installed using composer or a website which is installed from a downloaded zip file] should not have any need for the various development requirements - such as phpunit and other dev tools. Instead of requiring installers to remember to pass --no-dev to the commands - these options can be defined in the composer.json file [ie have a composer.json.dev and a composer.json.prod file - leave composer.json as it is in the github repository, but during the build process swap for the production/safer composer file].

By the same token, using the development version of vendor libraries adds yet more insecurity. A quick search for $_GET variables shows a large number of unsafe usages in the folder lusitanian/oauth/examples - with no fault to the library author. Examples and demo code do not need to be secure - it is expected that implementors would not deploy such code.

For the oath examples, they are protected by the htaccess files from direct access - however if Magento2 can be tricked into loading one that leads to security issues. Due to the reliance on the composer autoloading mechanisms such mischief is possible in theory.

[buskamuza]

Hi @gamort , thanks for reporting it. We're looking into the issue and your suggestions.
Internal ticket is MAGETWO-52095

[jameshalsall]

The composer --no-dev flag is designed exactly for this purpose, saying that you shouldn't have to require the installer to pass that option when building for production is nonsense.

Example code in the vendor library isn't a security issue over HTTP unless you're including / executing it as part of your Magento store.

[piotrekkaminski]

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

[gamort]

@jameshalsall makes a curious statement.
"saying that you shouldn't have to require the installer to pass that option when building for production is nonsense."
 
Just wondering if you still stand by that comment or if you have learned a bit more about how Composer works and how it is used by Magento?

Various magento command line tools invoke composer install/upgrade internally.  I'm not aware of any way to pass a command line parameter to composer such as --no-dev when executing a Magento CLI command such as php -f bin/magento setup:upgrade   To my knowledge the only way to do it is to make it the default by specifying in the composer.json
 
Now, my understanding of common exploits discovered in the wild frequently occur when a php script accepts unsanitized data from $_GET and $_POST and then pass it to other function calls where cleverly crafted strings might do things like: send the contents of the config.php file containing the database user and password to an external site.  So to me a directory such as https://github.com/Lusitanian/PHPoAuthLib/tree/master/examples which has over 50 files that use unsanitized inputs to do who knows what seems dangerous.   I have not audited all those files since I think it is an unnecessary risk.   Have you had a chance to audit those files?
 
A complete idiot might make the claim that since access to the vendor directory is restricted by the htaccess file it's not an issue.  But anyone with a year of experience using the command line would of course have executed:
find -name ".htaccess"
 
So they would know that there are a large number of .htaccess files in the vendor directory which completely override the root level htaccess file.   Since composer will install and upgrade libraries from the github repository of the library author there is no guarantee that a package will not be replaced by malicious code.  If it installs abandoned code, it will install malicious code:
Package videlalvaro/php-amqplib is abandoned, you should avoid using it. Use php-amqplib/php-amqplib instead.
Package fabpot/php-cs-fixer is abandoned, you should avoid using it. Use friendsofphp/php-cs-fixer instead

It is not possible for a site owner to fix this permanently, since when Magento2 is upgraded, the composer.json file in the webroot will be replaced by the latest magento2 copy - deleting those fixes.
 
A cronjob to fix these issues when they keep reappearing is a good idea.

[jameshalsall]

@gamort I think this was me misunderstanding Magento's installation procedure at any depth.

On the vendor stuff, this is only a security problem if your web root is the Magento root folder, it should always be the pub folder. Any files outside of the web root are fine providing they aren't included in any of the files inside the web root. This is a huge issue for Magento, it should not have a front controller (index.php) in the root directory as it poses security problems.

Another point worth making is that you should be packaging your release on a build server, and removing any unnecessary files (like the dev directory) before deploying it.