Composer: Downloading Random Code Is Not A Security Vulnerability?

Update: A fix which prevents Composer from locally installing packages not explicitly referred to by your root composer.json, or not explicitly referred to by your dependencies, has been committed to the Github repository: https://github.com/composer/composer/issues/2690. This change is live, so make sure to update your copy of Composer when possible. If you still experience any unexpected replaced/provided packages, be sure to open an issue.

A few days ago, I spotted a post floating past on Twitter entitled “Composer: Replace, Conflict & Forks Explained”. There has been some recent complaining about Composer downloading the wrong packages as dependencies due to its “replace” feature misbehaving, so seeing something hit the blogs was not unexpected. The issue has been encountered previously so having it reoccur caused some consternation. The article states:

Recently there has been an increase of cases in which Composer installs a fork of a package instead of the package the user expects. […] First of all, this behavior is not a security issue in Composer.

The problem here is quite simple. A user defines a composer.json file that requires the package bloggs/framework. Someone else creates a package on Packagist.org called evil/framework whose own composer.json states that it replaces bloggs/framework. Next, a group of poor random victims, potentially thousands, use composer to install applications with a dependency on bloggs/framework. Composer does some internal wizardry and installs evil/framework when certain conditions are met. The victims didn’t request evil/framework but they get it anyway.

If a piece of software, upon running, downloads files under the control of a potential attacker it is called a Remote File Inclusion (RFI) vulnerability. If the new file finds itself in a scope where it is executed, it’s also a Remote Code Execution (RCE) vulnerability. Two vulnerabilities for the price of one.

These definitions of two vulnerabilities apply to all software…except Composer. It’s possible that it’s not a security issue in Composer because it is in Packagist.org. That works too perhaps.

If your dependencies lead to a conflict with a package Composer may decide to install a fork instead which does not have the same conflict. If you notice that an unexpected fork is installed when running `composer update` you can debug the dependency problem that lead to the fork installation. Use the conflict key in your composer.json to blacklist the fork.

So don’t worry. This is all YOUR fault for not debugging, using conflict keys and assuming composer wouldn’t install potentially hostile code you never requested.

The mitigating manual checks described in the referenced blog post are not documented in the Composer manual as security concerns. This is the first time that these suggestions are being clearly communicated, so I assume many readers are not currently following them. They are also deeply flawed. Obviously, they are flawed because expecting people to do things that they never suspected were necessary is futile, but there’s also no attempt to elaborate on their assumed effectiveness.

Consider a scenario where an attacker decides that all this conflict blacklisting crap needs to be defeated for the few actually using blacklisting. They will immediately try two things:

Flood Packagist.org with replacement packages. Try getting an attacker to voluntarily remove packages or have Packagist.org’s team manually weed them out. That may a) create frustration to end users if the attack succeeds, b) impose a denial of service on those trying to block illegal replaces, and c) destroy trust in Composer as a secure platform.

They will not use obvious names. Would you trust something called fpotencier/twig? What about phpfig/log, synfony/console or laraval/framework? There are an infinite number of innocent looking and trustworthy package names to apply to the infinite number of false packages an attacker could setup. Longer names are easier to fudge and less likely to draw attention from the big frameworks unless they break CI builds.

We understand that Composer’s behaviour regarding forks and package replacements is unintuitive. So I’ve proposed a number of changes yet to be implemented to the handling of replace & provide on our issue tracker at https://github.com/composer/composer/issues/2690.

So, the unintuitive feature which is definitely not a security vulnerability is going to be changed. Will its ability to inject hostile code be documented? Will it have an optional off switch added to the command line options?

Only packages which match a dependency by name (ignoring replaces/providers) or a dependency of any potential dependency identified recursively by name will be considered for installation. The error reporting should further query for alternative replacers/providers which the user can require in the root package to satisfy the dependency.

The feature is being removed at some future unspecified time.

There’s more though. The Packagist.org folk are also manually deleting packages which are found to be replacing valid packages improperly and labeling it as an abuse of Packagist. If it’s an abuse – where are the Packagist.org automatic checks to prevent it? If it’s an abuse – how can it possibly be a “feature”? If it’s an abuse – isn’t that also another one of those things that are definitely not a security vulnerability?

Anyway I deleted the offending fork, @mlebkowski @nediam please take notice and use https://getcomposer.org/doc/05-repositories.md#vcs instead of abusing packagist to fork packages. If it’s a real fork you intend to maintain you can submit it to packagist but then you should not use replace and you should definitely add a note explaining what your version does differently.

Saying one thing, but acting like it’s the other thing you don’t want people to call it, makes me think it really is the other thing. Probably because it is. Users can fall victim to a replace and it’s called “unintuitive”, but if a package states that it replaces something that might lead to the unintuitive behaviour, it’s an abuse.

Packagist.org hosts somewhere just short of 25,000 packages. That almost beggars belief in a community barely out of the shadow of PEAR’s dominance. Composer has become an integral part of how we develop and distribute software. It simply doesn’t fit well with me that Composer can have security issues of this extent, over extended periods of time, and then see blog posts denying their very existence under the cover of “users are holding it wrong” while fixing them anyway in public Github issues for the whole world to see.

The reason why I absolutely despair at people denying the obvious is that it’s a bad habit to fall victim to. When you can deny a security vulnerability exists, even when it obviously does exist, you can justify not fixing it or undertaking endless bikeshedding. In this particular case, there’s finally pressure there to get something done on a problem that has continued occurring despite repeated complaints.

This is not the first time an issue has been openly discussed concerning Composer’s attitude to security. Composer is too intrinsically a part of PHP’s ecosystem to be allowed to take security issues so lightly. Since users are obviously exposed, this issue needs to be fixed as soon as possible. Preferably by yesterday.

In the meantime – readers should do as the referenced blog post says. The critical part is checking what you actually installed (I’m assuming dry runs followed by installs are susceptible to race conditions with Packagist.org’s update cycle), not running any installation scripts and being 100% certain that your composer.lock file is free of creative spelling errors.

This is a fantastic point. Drives the point home completely that it is a security vulnerability. You don’t even get a chance to review what was updated before the code gets executed. The only practical mitigation technique currently available is to do a dry run first.

I’m not going to argue with the fact that it sucks that replace can be abused. But I don’t think you present the full picture (and I assume it’s due to a misunderstanding). First of all we did take steps in the past to mitigate this already: when multiple replacer packages match the ones of a similar vendor namespace will be preferred.

If we take for example zendframework/zendframework dev-develop, it replaces zendframework/zend-barcode dev-develop. So if you request zendframework/zend-barcode dev-develop and it didn’t exist, you would get zendframework/zendframework, even if foo/bar replaces it as well. The problem now is if you require the barcode package in dev-bonkers for example, then none of the versions of zendframework/zendframework replace dev-bonkers, so if foo/bar says it replaces * it will win in this case and be installed.

So to sum it up:

– The replace feature is legit, and necessary, and used by many high profile packages. We can’t just remove it, and we can’t block it entirely. Yet when someone marks their forked package as replacing another one, it *is* an abuse of the feature since they don’t replace the same code. We can’t easily check this automatically though. The only way to improve it on packagist’s side is to restrict replace to same vendors for example, but that would also require that people can own a vendor namespace. Those would be two nice features, but they take time to implement, and we all only have so much of that.
– Unless we introduced a regression, the only way to get some code injected in this way is if your requirements are invalid and some abusive package has overly-greedy replace constraints. The simple fact that someone adds an exploit package would not lead to all users of the replaced package to be vulnerable.
– It’s overall a bad situation, and we are doing what we can to improve things. But replace itself is not a security vulnerability, it’s a feature.

http://evertpot.com/ Evert

That’s ridiculous Jordi. I understand that the feature is used for legit reasons, but anything that automatically runs ‘composer update’ is susceptible to some really bad stuff. It’s only a matter of time before people will start abusing en-mass and deploy a botnet by intercepting half a dozen popular packages.

Replacing a package, and allowing a replacement to happen should be authorized by the package that’s being replaced, not by the replacer.

Cam Spiers

While I agree, and do think this issue is a security concern. Can we **please** spread the idea that it is a very bad practice to run “composer update” in automated systems? Because it is, and was even before this potential attack vector was discovered. Only “composer install” when a lock file is present should be run. I do understand that there many be motivations to do such a thing, but it is a bad idea for reasons in addition to this exploit.

http://evertpot.com/ Evert

Well right now it’s a bad idea to run composer update in a development environment as well… Not a great situation.

Cam Spiers

Totally agree and don’t understand the reluctance of certain people to not recognise it.

http://blog.astrumfutura.com Pádraic Brady

Running “php composer.phar install” may install hostile executable code of unknown origins without the permission of the user.

It is a Remote Code Execution vulnerability. End of story.

You can refer to it as a feature all day long, but that boils down to you having made a terrible design decision and, because it WAS a design decision, refusing to admit it was a mistake because it was done deliberately at the time. I agree – it is a feature. It’s also a security vulnerability. The two are mutually exclusive notions. You can’t use one truth to hide the other truth.

If it’s a feature, it needs to go. If it’s a failure of Packagist.org controls, they need to be added. If you want to restrict it, then restrict it to the parties owning the replacing package – they do register accounts do they not? What you are not understanding is that this is a fatal problem. It has sat around in Composer as public knowledge (the last issue I saw referenced is 4 months old but we both know this goes back years), leaving users susceptible to RCE attacks, and YOU HAVE NOT FIXED IT. How hard and long do people have to scream from the rooftops before it does? You have folk with a background in security refusing to touch Composer with a barge pole for a good reason. It’s not the first time you have not fixed a security issue, and having one presented as not being a vulnerability is not acceptable in such an important PHP project.

I can understand that your time is limited, but please – drop every other Composer related coding task that is not essential and get this issue resolved.

Tomáš Fejfar

One real case scenario is worth 1000s words. Probably it would be more powerful if you create a script that will randomly create new replaces for random packages and release the chaos! That may convince @jordiboggiano:disqus that this is critical even though he may not consider it “security issue” 😉

http://blog.astrumfutura.com Pádraic Brady

It’s not so easy to control the trigger timing since it needs a dependency conflict to occur first. They do occur and may be more likely with popular packages (more opportunities to get conflicts). Also, an actual attack would be unethical ;). A POC would need to run in isolation – it’s not really needed though since we have real world examples already so proving it works is unnecessary.

http://dongilbert.net/ Don Gilbert

I like the idea of allowing users / organizations to “own” a vendor namespace. What would it take to build that functionality? Has that been considered and a plan laid with just lack of time to execute? Or is it just a basic concept at this point that needs fleshing out?

An alternative workaround would be to add a flag to specify –no-forks or –no-replaces or something like that until this was worked out.

Cory Comer

I think this seems like a pretty sane approach as well. Take Composer and Packagist out of the trust game and allow whoever owns the vendor namespace to decide what forks/repos replace what. You still get plenty of flexibility with a lot more control. This functionality is nearly assumed as it is today by this comment: “…when multiple replacer packages match the ones of a similar vendor namespace will be preferred”.

If someone has a fork and they wish to replace a package, it shouldn’t be too much work to communicate with the maintainers/namespace owners to get the fork under the vendor namespace it wants to replace. If that’s not possible because the package isn’t being maintained any more or there’s a conflict between the fork owner and namespace owner, why would anyone want that fork replacing the original package anyways?

Anonymous

I had assumed that if my requirements were invalid, the composer update process would fail.

I am uncomfortable with a developer unknown to me being able to decide “their forked package of what I requested should be used.” I need to know that what I specified should be included was included.

Unless ‘replace’ can be turned off, I am sorry to say this, but it calls into question the integrity of the Composer’s update process. I’d need to add some post-update process to ensure the packages included were what I requested.

It’s still unclear to me how this would be useful to anyone. But, if it is useful somehow, it must be a very small section of developers who would want this to happen.

Sorry to pile on, but I can’t see this as a feature, just as a problem.

Jordi Boggiano

Say you are using symfony/symfony and you also want to use phpunit/phpunit in the project. phpunit/phpunit depends on symfony/yaml. Since you’re using symfony/symfony already and that package replaces symfony/yaml (since the yaml component is included in the full symfony package) phpunit will be installed just fine and symfony/yaml will not be installed. There are other use cases with forks but those tend to cause problems right now when people don’t use it wisely, and that’s what we are planning on fixing. So this is a feature that many many people rely on implicitly, they just don’t know it’s there. Similarly most people I see commenting here clearly do not know what replace does or do not fully understand the whole thing.

Cory Comer

So when Composer is building the dependency list the package symfony/symfony says “we can replace requests to install symfony/yaml”, Composer takes note and continues. Composer gets to the phpunit/phpunit package, finds the symfony/yaml dependency, sees that symfony/symfony claims it replaces symfony/yaml in the event there’s a conflict, it doesn’t bother adding symfony/yaml as a separate dependency and moves on without any sort of validation or checking or user control?

This “feature” is confusing, I think partly because I cannot comprehend why another package should have the right to intercede for another without some sort of control system in place, but I’m trying to understand the situation 100%.

Jordi Boggiano

It is needed in two cases:

– To avoid waste like in my example above, because installing symfony/symfony + symfony/yaml together makes no sense you’d just have the same code twice.

– To use forks of things that are required. Say you again want to use phpunit but you also want to use cory/yaml which is your fork of the yaml component including important changes to the code. If cory/yaml says it replaces symfony/yaml all is well it’ll be installed and phpunit’s dependencies will be satisfied. If it does not replace it, then symfony/yaml will be installed as well, which may or may not be a problem in this case. Maybe symfony/yaml would have dependencies conflicting with your other dependencies, in which case it’d be an unsolvable case without replace.

avoiding to install symfony/symfony and symfony/yaml at the same time is not only about avoiding waste. If you have a different version for Yaml and the fullstack framework, it might create very weird bugs, because you would have 2 versions of the classes in the codebase (and you could end up with half of the Yaml classes loaded in one version and the other half in a separate version, which is likely to break things)

Christophe Coevoet

avoiding to install symfony/symfony and symfony/yaml at the same time is not only about avoiding waste. If you have a different version for Yaml and the fullstack framework, it might create very weird bugs, because you would have 2 versions of the classes in the codebase (and you could end up with half of the Yaml classes loaded in one version and the other half in a separate version, which is likely to break things)

Anonymous

Thanks Jordi – I get the point now. Thanks.

Anonymous

Commit your composer.lock and use composer install. You should rarely ever use composer update, as the whole point is “get me the latest version of code that is fitting these requirements.” That could be a whole range of things and some of it could be dodgy, so my advice goes a step further:

NEVER run “$ composer update” at all. EVER.

Run “$ composer update specific-package” then check the code. Run your tests, see if it works, see if its legit and commit your lock file.

Unless I’m wrong, you can’t get hacked doing this, and it stops you getting random updates you aren’t expecting either.

http://ocramius.github.io/ Marco Pivetta

You can also run composer update --dry-run, which will basically show what is going to happen.

composer update should still happen daily on dev boxes to discover breakages eagerly.

gggeek

Q: how do you check the output of “dry-run” for this, manually?

Take into account an app which includes a cms which includes a framework which…. the list of packages you get is staggering (e.g. 11.192 files in /vendor for ezpublish 5.2).

Are there clear indicators that your installed 4th level dependency foo/bar is an uncalled-for replacement for baz/bar, which was specified by the 3rd level dep?

Side note: with such a staggering amount of php code onboard, I think it might be as easy to insert some malicious code in what seems to be a “legit” package or a package fork and trick users to install it voluntarily using their main composer.json (“look, the twig/twig package on my github repo is shinier!”) . And what about attacks against packagist.org? And out-of-date dependencies with known security problems?

There is a lot of trust in the ecosystem already in place….
Not the fault of composer of course, but as it makes depending on other libraries such a breeze, it indirectly encourages “risky/promiscuous” behaviour.
Maybe package signing and blacklist servers will have to be made mandatory at some point.

http://ocramius.github.io/ Marco Pivetta

> Q: how do you check the output of “dry-run” for this, manually?

Yep, manually

> Take into account an app which includes a cms which includes a framework
which…. the list of packages you get is staggering (e.g. 11.192 files
in /vendor for ezpublish 5.2).

It’s still not a big deal to see what a “composer update” will cause. If you do it regularly, you get mostly 2 or 3 packages changed per day (that’s my average on very large applications)

> Side note: with such a staggering amount of php code onboard, I think it
might be as easy to insert some malicious code in what seems to be a
“legit” package or a package fork and trick users to install it
voluntarily using their main composer.json (“look, the twig/twig package
on my github repo is shinier!”) .

Yes, never denied that the issue is a security problem. I was just pointing out how to verify what is going to change in a composer update run

> And what about attacks against packagist.org?

Can’t do much about that except moving everything to a private satis repo. This question applies to any kind of repository anywhere – if the site is violated, then the attacker can play around with private keys and signing in general even if package signing was a thing.

> Maybe package signing and blacklist servers will have to be made mandatory at some point.

Blacklisting is notoriously a bad security approach. A whitelist is better

Cory Comer

That this has to even be said should be enough of a glaring red-flag to indicate this is a very serious problem with what has become the de-facto method of managing dependencies for PHP projects today.

There may have been edge cases with other similar tools like maven, gradle, npm, gems, apt, pip, bower, grunt, etc… but I cannot remember a time where the consensus in a community was to be so afraid of updating your dependencies that prominent members would advise against it completely, especially due to the potential for mysterious code to be installed where you might not want it.

Anonymous

Generally speaking I always use this approach. I’d do it with or without this weird fork logic, for the same reasons. I do it in Ruby too.

This fork stuff only adds the “make sure a shitty version has not been installed when you run “$ composer update specific-package”, and if you’re using dry-run then thats not hard at all.

Cory Comer

I certainly have no problem with recommending against updating vendor dependencies for the reason of not getting a shitty version of something, I just took issue with recommending against it due to the possibility of code from someone you did not ask for getting installed; that seemed … broken?

Either way, moot point, the “non-security issue feature” has been changed and we can go back to complaining about other things, yay!

http://androidflavor.com James Frost

I don’t think so

http://www.swiftarrow.net/ Maid Agency

I believe security of networks and stuffs are definitely the trend in the future, we should focus more on security and patch up all the vulnerabilities in any system.