Cookies op Tweakers

Tweakers is onderdeel van DPG Media en maakt gebruik van cookies, JavaScript en vergelijkbare technologie om je onder andere een optimale gebruikerservaring te bieden. Ook kan Tweakers hierdoor het gedrag van bezoekers vastleggen en analyseren. Door gebruik te maken van deze website, of door op 'Cookies accepteren' te klikken, geef je toestemming voor het gebruik van cookies. Wil je meer informatie over cookies en hoe ze worden gebruikt? Bekijk dan ons cookiebeleid.

Meer informatie

Onderzoeker vindt rce-lek op site van php-repository Packagist

Onderzoeker Max Justicz heeft een kwetsbaarheid in de dienst van Packagist gevonden, een populaire repository voor php-packages. Het inmiddels gedichte lek maakte het volgens Justicz mogelijk om op afstand code uit te voeren op de Packagist-server.

De onderzoeker schrijft in een blogpost dat hij in staat was om via de uploadfunctie, waar gebruikers van Packagist nieuwe packages kunnen aanbieden, een commando in te vullen in het veld waar normaal gesproken een url voor een package ingevoerd dient te worden. Bijvoorbeeld een link naar een Git-repository of een ander versiebeheersysteem.

Daarbij was het zo dat de vervolgens op de server uitgevoerde commando's p4 en svn, respectievelijk voor Perforce en Subversion, geen escape gebruikten voor de url. Daardoor werd het commando uitgevoerd. Volgens de onderzoeker heeft het Packagist-team inmiddels een patch doorgevoerd.

Packagist is een dienst die packages aanbiedt voor de php-dependency manager Composer. Volgens zijn eigen statistieken host de dienst bijna 200.000 packages en worden er maandelijks ongeveer 400 miljoen per maand geïnstalleerd.

De manier waarop het uitvoeren van commando's mogelijk was, afbeelding via Justicz

Wat vind je van dit artikel?

Geef je mening in het Geachte Redactie-forum.

Door Sander van Voorst

Nieuwsredacteur

29-08-2018 • 14:26

19 Linkedin

Reacties (19)

Wijzig sortering
Ik schrok al even en dacht dat het wellicht wat verder ging, bijvoorbeeld richting Composer of zo. Hoewel vervelend voor Packagist, stiekem (in a way) toch wel blij dat het enkel hun omgeving trof. Fijn dat het lek intussen ook gedicht is!
Ligt eraan natuurlijk; Als er aan de hand van deze lek RCE mogelijk is dan is het ook mogelijk, gezien het gaat om de server waarop de GIT repo wordt geplaatst, dat andere packages binnen packagist zijn aangepast, en misschien ook composer zelf wel. Hiermee is het dus in theorie mogelijk dat iedereen die composer gebruikt een risico loopt op het moment doordat er aangepaste bestanden zijn gedownload. Ik weet niet exact hoe hun infrastructuur is opgezet, maar ik mis toch wel een beetje diepgang vanuit zowel Packagist als vanuit de publicatie van de onderzoeker.
Aangezien het lek ook intussen gedicht is, is denk ik daarom de publicatie ook niet zo diepgaand? ;)
Ondertussen wel inderdaad, maar:
- Het is op de Composer branch aangepast i.p.v. op de packagist branch wat bij mij het idee geeft dat Composer zelf dus ook kwetsbaar is in combinatie met andere lekken (om een of andere reden krijgt iemand het voor elkaar om de composer.json aan te passen en een custom repo toe te voegen, ik noem maar iets simpels).
- De commit is 4 dagen geleden gedaan (rechtstreeks op de master branch :s ) en de laatste release is van 13 dagen geleden, dus in de laatste release zit deze fix nog niet.
- Dit lek kan natuurlijk al lang en breed misbruikt zijn op hun servers maar er wordt niets gezegd over of ze dit onderzocht hebben, en waarom het wel of geen verdere impact heeft.
Zoals hier te zien is zal dit in versie 1.8 pas gefixt zijn:
https://github.com/composer/composer/compare/1.7.2...master welke nog niet uit is.

[Reactie gewijzigd door mrdemc op 29 augustus 2018 15:44]

Tja, een security leak wil je sowieso zo snel mogelijk live hebben. Dat dit op master branch is gedaan, is wellicht niet helemaal "netjes", maar gezien de severity, de haast en het een hele kleine fix is, zie ik er ook niet al te veel kwaad in.
Klopt, maar dat kan dat ook prima op een aparte branch en een pull request (een fix voor een bug wil je ook laten nakijken via een review zodat je niet per ongeluk een nieuw lek introduceert of de bestaande eigenlijk niet fixt).

Daarnaast is de huidige composer.phar die via de getcomposer.org website wordt aangeboden dus wél aangepast, maar de composer.phar via de release tab in github niet. Nu is die via de getcomposer.org de meest gebruikte verwacht ik, maar naar mijn idee niet zo heel netjes op het op die manier te doen.

En de huidige composer.phar die via 'composer self-update' wordt gedownload is ook de niet gepatchte versie!

[Reactie gewijzigd door mrdemc op 29 augustus 2018 16:32]

Dan ben ik nu eigenlijk wel benieuwd hoe er escaped werd, hoe dat nu gebeurt en hoe het misbruikt kon worden.

Wat ik uit de patch haal:
return 0 === $processExecutor->execute('p4 -p ' . $url . ' info -s', $output);
is geworden:
return 0 === $processExecutor->execute('p4 -p ' . ProcessExecutor::escape($url) . ' info -s', $output);
Dat lijkt alsof er niet escaped werd? Dan is het eerder vreemd dat dit niet eerder misbruikt is?

[Reactie gewijzigd door JDVB op 29 augustus 2018 15:29]

ProcessExecutor::escape() op Github lijkt in elk geval duidelijk aan te geven wat er escaped wordt en hoe. ;)

Overigens is het ook gedeeltelijk een uit Symfony 4 verwijderde functie, dat zal vast ook niet zonder reden zijn, lijkt me.

[Reactie gewijzigd door CH4OS op 29 augustus 2018 16:04]

Het ging mij voornamelijk om het verschil en hoe dat opgelost is. Van geen escape naar wel een escape is dan nogal duidelijk.
Ik zie bij de patch ook geregeld bij een variabele een object van class IOInterface mee komen, wellicht zit daarin dus ook wat gepatched, anders hoefde dat ook niet te veranderen. ;)
Dat instantiërt de de door jou gelinkte klasse met $this->io, welke niet gebruikt wordt bij het escapen (de functie die je linkt).

Het heeft wel met de execute functie te maken:
if ($this->io && $this->io->isDebug()) {
Om gemakkelijker te kunnen debuggen lijkt die io dus meegegeven te zijn?

edit:
Het artikel is ondertussen aangepast en schrijft nu dat de url inderdaad niet escaped werd. Dat stelt me toch niet heel gerust dat een zoveel gebruikte service dergelijke basale dingen niet af had gevangen. Ik had eigenlijk verwacht een escape methode voorbij te zien komen die nu onveilig blijkt te zijn, waarop ik van plan was te zoeken of ik ergens vergelijkbare escapes gebruik die dan ook vervangen moeten worden. Dat blijkt nu dus niet aan de orde.

[Reactie gewijzigd door JDVB op 29 augustus 2018 16:05]

De beveiligingsonderzoeker stelt:
In particular, I think it is a security anti-pattern to have application build pipelines pull fresh downloads of packages from upstream servers on every build if the packages are not expected to change. If for some reason you have to do this, you should pin dependencies using a cryptographically secure hash function.
Normaliter zet ik mijn dependencies niet in mijn repositories omdat deze daar niet thuis horen.
Het mooie aan composer vond ik altijd dat ik zelf gemakkelijk kan kiezen of ik minor updates vanzelf laat meekomen om daarmee de 'compromised packages' automatisch bijgewerkt te krijgen naar de meest recente versie.

De blog die gelinkt is laat zien hoe vaak en hoeveel ernstige kwetsbaarheden bij de upstream package managers gevonden worden en dat deze dus inherent niet te vertrouwen zijn. Het idee om automatisch bijgewerkte pakketten binnen te halen om daarmee veiliger te zijn en om daarmee de code van die pakketten eenvoudig buiten de eigen repositories te houden kan zich dus eenvoudig tegen mij keren. Het niet bijwerken van pakketten is echter ook een no-go en handmatig alle commits op alle pakketten die gebruikt worden langslopen etc. is ook niet een optie.

Ik ben ondanks dit toch geneigd door te gaan op dezelfde voet als dat ik al deed, via composer de nieuwste versies binnen blijven halen. Maar de afwezigheid een escape op iets basaals als een door de gebruiker opgegeven url doet mij toch sterk twijfelen. Ik zie alleen geen haalbare alternatieven.

Iemand anders wel?
Packages gaan toch niet automatisch naar een nieuwere minor versie, die worden specifiek gelockt in de composer.lock?
En die staat bij mij in de .gitignore.
Het opnieuw ophalen van eerder opgehaalde repositories is ook niet waar ik me zorgen om maak, die staan wel in de local-cache.
Waarom zet je de lock file in de .gitignore? Hierdoor krijg je dus versie verschillen als je met productie en staging omgevingen werkt. Beste is dus de lock file juist wel in de repository te laten zodat composer install altijd de juiste versies gebruikt.
Als je een dienst als deze weet de kraaken, dan kun je een behoorlijk botnet van websites opzetten. Het klink als een klein foutje, maar de impact kan onmeetbaar zijn.
Het staat je vrij om een apart topic te starten in Geachte Redactie waarin je de auteur dan ook even "tagged".

Op dit item kan niet meer gereageerd worden.


Nintendo Switch (OLED model) Apple iPhone 13 LG G1 Google Pixel 6 Call of Duty: Vanguard Samsung Galaxy S21 5G Apple iPad Pro (2021) 11" Wi-Fi, 8GB ram Nintendo Switch Lite

Tweakers vormt samen met Hardware Info, AutoTrack, Gaspedaal.nl, Nationale Vacaturebank, Intermediair en Independer DPG Online Services B.V.
Alle rechten voorbehouden © 1998 - 2021 Hosting door True