[php] Is mn uploadscriptje veilig

[pim]

Het lijkt me wel, maar wil ik voor de zekerheid even controleren.
Niet dat dadelijk iemand php bestandjes gaat uploaden en uitvoeren..

PHP:

<?
 
if ($action == "Submit") { 

    If($MyFile != "none") {

    $extensie = substr("$MyFile_name", -5,5);

    if (preg_match("/.gif/", $extensie) || 
        preg_match("/.jpg/", $extensie) ||
        preg_match("/.png/", $extensie) ||
        preg_match("/.mpeg/", $extensie) ||
        preg_match("/.mpg/", $extensie) ||
        preg_match("/.jpeg/", $extensie)) { 
    
        copy($MyFile,"$MyFile_name");
        unlink($MyFile);
        echo "Je hebt net het bestand [ <b>$MyFile_name</b> ] 
met een grootte van [ <b>$MyFile_size</b> ] geupload";
        } else {
        echo "Je kunt alleen bestanden uploaden met de volgende extensies: 
.gif .jpg .jpeg .png .mpeg .mpg<br><br>"; 

}
}
}
?>

[Verwijderd]

Zit wel goed volgens mij

[KlaasB]

Volgens mij kun je de extensie ook op een andere manier testen. (Plaatjes zowiezo miet GetImageSize)

[chem]

nog leuker: maak een .htaccess file aan met een 'lock' op .php...

[frosty1878]

Op zaterdag 11 augustus 2001 10:42 schreef KlaasB het volgende:
Volgens mij kun je de extensie ook op een andere manier testen. (Plaatjes zowiezo miet GetImageSize)

$MyFile_type ($userfile_type) geeft mime types weer. "image/gif" etc..

[Verwijderd]

Op zaterdag 11 augustus 2001 11:11 schreef chem het volgende:
nog leuker: maak een .htaccess file aan met een 'lock' op .php...

hehehe lijkt me heeeel wijs

[Infinitive]

Misschien dat de volgende fout erin zit: je test niet of $MyFile_name uit de $image_track_vars[] komt. Dus als ik geen afbeelding meestuur, maar wel via de http-query een fake plaatje meestuur:

http://scriptje.php?MyFile=/etc/passwd&MyFile_name=jantje.gif

dan slagen de tests, en wordt er een copie van /etc/passwd opgeslagen als jantje.gif. Dit plaatje kan ik openen en download&rename ik naar .txt en kan dan zo je /etc/passwd filetje bekijken (of je ./index.php of iets dergelijks).

Verder zou ik als extra check nog kijken of het geuploade filetje wel in de upload/tmp-directory staat (dat weet je zeker dat als er geknoeit is met de $MyFile_name dat het alleen bij onschuldige files in de tmp/upload-directory kan).

[Infinitive]

code:

unlink($MyFile);

Waarom unlink je die file, dat is volgens mij niet nodig...?

[tomato]

Je check op extensie is een beetje raar. Ten eerste check je niet op een punt zoals je waarschijnlijk wel bedoelt (. in een regex == alle karakters behalve newline). Een .htaccess neerplanten is nog niet mogelijk, maar stel dat je een plaatje met denkbeeldige extensie '.ess' zou toelaten, dan zou het toeval minder leuk zijn. Het is nu trouwens wel mogelijk een bestand met naam '.htjpg' te uploaden (wat niet gevaarlijk is, maar wat niet meer van buitenaf opgevraagd kan worden. Verder is het ook mogelijk een bestand zonder extensie up te loaden.
Ik zou er naast deze puntjes nog veel meer restricties op leggen, dan krijg je bijvoorbeeld zoiets:

code:

/^[a-z0-9_-]{1,7}\.(?:gif|jpg|png|mpeg|mpg|jpeg)$/i

Maar eigenlijk zou ik het zowiezo nooit zo doen, ik zou het bestand onder een zelfgegenereerde naam opslaan. Als de originele bestandsnaam toch nog van belang is, sla die dan ergens anders op.

[pim]

8 jaar later is de server gehacked waar dit scriptje op draaide..
Iemand had een 'filename.pjpeg' geupload wat uitgevoerd werd als .php bestand..

Ik wist eerlijk gezegd niet eens dat .pjpeg als php geparsed werd

[RobIII]

pjpeg = progressive jpeg; die extensie kwam door de check door wat tomato indertijd aankaartte. Waarom dat bestand door PHP gehaald is weet jij beter dan wij Je doet echter, zoals ik in je TS zie, enkel een check op extensie en dat is idd nogal "2001" De extensie garandeerd namelijk niets m.b.t. de inhoud van de file.

[ Voor 83% gewijzigd door RobIII op 06-07-2009 19:32 ]

[AtleX]

Wordt het normaal gesproken ook niet, maar als je in de serverconfig aangeeft dat dat wel moet doet 'ie dat wel natuurlijk.

[RobIII]

AtleX schreef op maandag 06 juli 2009 @ 19:26:
Wordt het normaal gesproken ook niet, maar als je in de serverconfig aangeeft dat dat wel moet doet 'ie dat wel natuurlijk.

Daarom zeg ik ook "weet jij beter dan wij"; dat kunnen wij namelijk niet zien, hij wel

[AtleX]

Uhm ja, maar als ik een bericht typ kan ik niet zien dat jij er zonodig weer iets tussendoor moet typen.

[Voutloos]

Je hebt dus sowieso de fout mbt het matchen van de punt (zie post tomato) laten zitten...

(los van dat extensie checken niet al te grondig te noemen is)

[ Voor 27% gewijzigd door Voutloos op 06-07-2009 19:30 ]

[Cartman!]

Die punt, had je die niet moeten escapen in je regex?

Wat ik altijd doe met imageuploads is ze proberen te parsen door image_jpeg/gif/png functies. Als dat niet lukt is het blijkbaar geen image. Met video's zou ik t zo snel niet weten.

edit: bedankt Voutloos

[ Voor 5% gewijzigd door Cartman! op 06-07-2009 19:32 ]

[Gomez12]

Hangt er vanaf hoeveel tijd je tot je beschikking hebt, maar videos recode / check ik meestal met ffmpeg.

Bijna altijd recoden, wat veel tijd en processorkracht kost, maar mits goed ingesteld uiteindelijk een besparing van bandbreedte oplevert...
Voor plaatjes idd gewoon kijken met plaatjes functies of je er iets mee kan...

[Verwijderd]

Volgens mij (als de . een \. wordt) matcht die regex op extensie ook op .jpg.exe of whatsoever, en bijvoorbeeld niet op .JPG.

[DJ-Visto]

pim schreef op maandag 06 juli 2009 @ 19:22:
8 jaar later is de server gehacked waar dit scriptje op draaide..
Iemand had een 'filename.pjpeg' geupload wat uitgevoerd werd als .php bestand..

Ik wist eerlijk gezegd niet eens dat .pjpeg als php geparsed werd

Best wel netjes na 8 jaar

[Verwijderd]

Cartman! schreef op maandag 06 juli 2009 @ 19:32:
Die punt, had je die niet moeten escapen in je regex?

Wat ik altijd doe met imageuploads is ze proberen te parsen door image_jpeg/gif/png functies. Als dat niet lukt is het blijkbaar geen image. Met video's zou ik t zo snel niet weten.

Je kunt beter file ofwel libmagic het bestand laten analyseren. Die is in staat om de meeste bestandstypen wel te herkennen.

[Barleone]

Verwijderd schreef op maandag 06 juli 2009 @ 20:08:
Volgens mij (als de . een \. wordt) matcht die regex op extensie ook op .jpg.exe of whatsoever, en bijvoorbeeld niet op .JPG.

inderdaad, daarvoor moet je forceren op .jpg aan het eind, dat is zoiets als:

"/\.jpg$/i"
\. is de punt
jpg zijn gewoon characters in die exacte volgorde
$ geeft aan dat het voorgaande teken het allerlaatste teken ...
... van de betreffende string moet zijn
de i achter / geeft aan dat het zowel hoofdletters als kleine letters mogen zijn

[pim]

Dat scriptje was tot 2 weken geleden niet door Google geindexeerd, en ik wist alleen de url..
Helaas was ik zo stom om de url kort geleden publiek te maken op een forum, toen is het geindexeerd door een bot en fout gegaan..

die pjpeg hoort niet door de PHP-parser te gaan.. Ik kan niet zien waarom dat gebeurd.. Zie niks in de apache2.conf, php.ini of .htaccess..

De extensie bepaald toch wat geparsed word op een Linux&apache server?

[Soultaker]

Verwijderd schreef op maandag 06 juli 2009 @ 20:12:
Je kunt beter file ofwel libmagic het bestand laten analyseren. Die is in staat om de meeste bestandstypen wel te herkennen.

Dat lijkt me in deze context nog een slechter idee. Als ik ergens in een plaatje wat PHP-code embed dan herkent libfile 'm (terecht) als plaatje, maar zal PHP die code wel uitvoeren (PHP emit immers gewoon alle data totdat 'ie <?php tegenkomt).

Je moet er gewoon voor zorgen dat geüploade bestanden nooit en te nimmer uitgevoerd worden (als PHP script, SSI, CGI, of wat dan ook). Een makkelijke manier is om de bestanden gewoon buiten de documentroot te plaatsen en ze alleen met een PHP script ofzo te serven, maar dat heeft weer als nadeel dat je alle caching e.d. voor je eigen rekening moet nemen. Het enige alternatief is ook de serverconfiguratie onder de loep te nemen, zodat je geüploade bestanden op basis van naam uit kunt sluiten van executie.

[ Voor 34% gewijzigd door Soultaker op 06-07-2009 20:58 ]

[Verwijderd]

pim schreef op maandag 06 juli 2009 @ 20:44:
Dat scriptje was tot 2 weken geleden niet door Google geindexeerd, en ik wist alleen de url..
Helaas was ik zo stom om de url kort geleden publiek te maken op een forum, toen is het geindexeerd door een bot en fout gegaan..

die pjpeg hoort niet door de PHP-parser te gaan.. Ik kan niet zien waarom dat gebeurd.. Zie niks in de apache2.conf, php.ini of .htaccess..

De extensie bepaald toch wat geparsed word op een Linux&apache server?

Ik ken niks van PHP, maar in Perl regex wordt een punt gezien als een joker teken. Dus als je op .jpeg matcht wordt pjpeg ook gepakt, als mede als ajpeg, bjpeg, cjpeg enz.

*shot in the dark* En/of misschien had de aanvaller PHP-code in de "bestandsnaam" gezet?

[ Voor 5% gewijzigd door Verwijderd op 06-07-2009 21:07 ]

[Voutloos]

Verwijderd schreef op maandag 06 juli 2009 @ 20:57:
Ik ken niks van PHP, maar in Perl regex wordt een punt gezien als een joker teken. Dus als je op .jpeg matcht wordt pjpeg ook gepakt, als mede als ajpeg, bjpeg, cjpeg enz.

Dat stond gelukkig al een keertje of 6, waaronder de eerste keer in 2001, in dit topic.

*shot in the dark* En/of misschien had de aanvaller PHP-code in de "bestandsnaam" gezet?

Eh nee, dat slaat nergens op. Zelfs met de meest achterlijke config krijg je dat niet voor elkaar.

[Verwijderd]

Voutloos schreef op maandag 06 juli 2009 @ 21:47:
[...]
Dat stond gelukkig al een keertje of 6, waaronder de eerste keer in 2001, in dit topic.
[...]
Eh nee, dat slaat nergens op. Zelfs met de meest achterlijke config krijg je dat niet voor elkaar.

/me sluipt geruisloos weg

[winkbrace]

RobIII schreef op maandag 06 juli 2009 @ 19:25:
... Je doet echter, zoals ik in je TS zie, enkel een check op extensie en dat is idd nogal "2001" ...

serieus.. Dit is echt heel erg grappig!

[Cartman!]

Verwijderd schreef op maandag 06 juli 2009 @ 20:12:
[...]

Je kunt beter file ofwel libmagic het bestand laten analyseren. Die is in staat om de meeste bestandstypen wel te herkennen.

Helaas heb je daar niks aan zonder shellrechten die vaak op sharedhosting-accounts niet aanwezig zijn. Daarom val ik altijd terug op de GD functies. Het enige wat ik hoef te doen is het volgende (ongeveer):

PHP:

try
{
    $image = new Image($source);
    $image->resize(400, 300);
    $image->outputType('png');
    $image->outputQuality(90);
    $image->save($destination);
}
catch(Exception $exception)
{
    // geen jpg/gif/png
}

Dan resize ik meteen de input altijd naar png, juiste kwaliteit en goede filesize zodat je ervanuit kunt gaan dat je altijd met png te doen hebt daarna. In de praktijk werkt dit erg snel en makkelijk.

Voor video gaat dit niet op maar als iemand echt video's kan uploaden dan moet je toch aan de ffmpeg waarschijnlijk en dan komt t ongeveer op t zelfde neer als bovenstaande code.

[Gomez12]

Cartman! schreef op maandag 06 juli 2009 @ 22:39:
[...]

Helaas heb je daar niks aan zonder shellrechten die vaak op sharedhosting-accounts niet aanwezig zijn. Daarom val ik altijd terug op de GD functies. Het enige wat ik hoef te doen is het volgende (ongeveer):

Houd aub wel in de gaten dat dit soort grappen tijd kosten ( en veel tijd ).

Het zou niet de eerste keer zijn dat ik hoor dat iemand een perfecte check/actie uitvoert die 1 sec duurt, terwijl zijn bezoekers die actie 1,01x per sec aanroepen. Dan zit je al vrij snel tegen een ddos vanuit je eigen klanten aan te kijken...

Bij een beetje traffic site kost dit soort checks al snel 100 / 1000x meer tijd dan de libmagic van cheatah. Tsja, en dan kan het beter werken ( doet het imho ook ) maar het kost wel ongelovelijk veel performance...

[Cartman!]

Daar ben ik me van bewust maar toch bedankt voor de info Het is een afweging die je moet maken. 9/10 keer heb ik geen libmagic beschikbaar en dan moet je toch iets natuurlijk En meestal bij projecten waarbij performance echt n issue gaat worden heb je wel beschikking over een goede (dedicated) server waar je gewoon ff kan bellen voor zaken als libmagic.