As part of T99084: Allow extensions to specify which MW core versions they support in extension.json, we will be re-using the composer LinkConstraint code, which has now been split out into a separate composer/semver library.
Tree: https://github.com/composer/semver/tree/0.1.0
Before adding to mediawiki/vendor, this needs a security review, so assigned to Chris.
| Project | Branch | Lines +/- | Subject | |
|---|---|---|---|---|
| mediawiki/vendor | master | +1 K -1 | Add composer/semver 0.1.0 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Declined | None | T88596 Improving extension management | |||
| Resolved | Legoktm | T99084 Allow extensions to specify which MW core versions they support in extension.json | |||
| Resolved | csteipp | T99086 Add composer/semver 0.1.0 to mediawiki/vendor |
At 37 kloc, this will take some time to review.
There are also open security issues with composer (https://github.com/composer/composer/issues/1074) that make me a little uncomfortable including it as a general library if we don't have a way of limiting how it's used.
Is there a chance you can carve out the code you need, and we'll just include those? Or can we add a patch throwing an exception if someone tries to use the self-update code?
I'm trying to come up with a way to address what you pointed out, but however I go about it, I come back to that I don't have sufficient knowledge of the threat model you try to counter in the software at hand. Please outline the threat model you have in mind regarding the self-update code.
@JanZerebecki, according to the issue (I haven't traced through the code enough to exploit it myself) when the self-update is used, it uses http (when openssl isn't installed) to download a new version of composer which it compiles and runs. If we provide composer as a library, and someone writes code to that uses the self-update from composer to update itself, it would be vulnerable too. I'm not sure how likely that is in this particular case, but statements like, "There was simply no expectation of security at the beginning" [https://github.com/composer/composer/pull/1092#issuecomment-8429586] don't inspire confidence in me that there aren't other similar issues. So limiting the scope of what we pull in would be a benefit, in my opinion.
which it compiles and runs.
AFAIK that is not true. Nothing gets compiled. Only the user runs something.
According to my reading of https://github.com/composer/composer/blob/bc45d9185513575434021527d7756420e9f4b2cf/src/Composer/Command/SelfUpdateCommand.php it may download a composer.phar from HTTP without verification. Under certain conditions it replaces $_SERVER['argv'][0] with the download. But it does not itself execute the downloaded code. Unless running http://de2.php.net/manual/en/phar.construct.php on a .phar does.
I think having that composer code available is equivalent to having the following PHP functions/features available: copy, stream wrappers, include. Certainly exploitable after someone where to combine them.
Under the normal circumstances of the user running php composer.phar self-update it is only exploitable because the user themselves runs php composer.phar ... again after that.
Do you think a security review of HHVMs and PHPs Phar::__construct($file) implementation for potential code execution from $file is worthwhile relative to other security reviews? Or was that done before we started running that code in production?
I'm asking upstream to see if we can split the specific code we need into a separate library so we don't need to pull in all of composer. Assigning to myself for now.
https://github.com/composer/composer/issues/3545 the general impression I'm getting is that upstream wants to do it, but they want to do it nicely with proper separation and stuff, and it's not high on their priority lists.
@csteipp: the code has now been split out into a *much smaller* library, so re-assigning to you for a security review.
Change 228132 had a related patch set uploaded (by Legoktm):
Add composer/semver 0.1.0