Page MenuHomePhabricator

Add composer/semver 0.1.0 to mediawiki/vendor
Closed, ResolvedPublic


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.


Before adding to mediawiki/vendor, this needs a security review, so assigned to Chris.

Event Timeline

Legoktm assigned this task to csteipp.
Legoktm raised the priority of this task from to Medium.
Legoktm updated the task description. (Show Details)
Legoktm added a subscriber: Legoktm.

At 37 kloc, this will take some time to review.

There are also open security issues with composer ( 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?

csteipp set Security to None.
csteipp moved this task from Incoming to Ready on the Security-Team board.

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" [] 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 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 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?

Legoktm added a subscriber: csteipp.

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. 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.

Legoktm renamed this task from Add composer/composer#bc45d9185513575434021527d7756420e9f4b2cf to mediawiki/vendor to Add composer/semver 0.1.0 to mediawiki/vendor.Jul 23 2015, 6:26 AM
Legoktm updated the task description. (Show Details)

@csteipp: the code has now been split out into a *much smaller* library, so re-assigning to you for a security review.

Nothing looks too worrying

Nothing looks too worrying

Thanks! Re-opening since the library hasn't been added to mediawiki/vendor yet :)

Change 228132 had a related patch set uploaded (by Legoktm):
Add composer/semver 0.1.0

Change 228132 merged by jenkins-bot:
Add composer/semver 0.1.0