[PHP4] Beveiliging upload class

[Bram77]

Ik heb een uitgebreide "AJAX" upload class geschreven voor PHP4 (750 regels code zonder comments). AJAX tussen aanhalingstekens omdat met AJAX niet geupload kan worden. Wat werkelijk gebeurd is dat een upload wordt gesubmit naar een hidden iframe. Waardoor de pagina dus niet gerefreshed hoeft te worden.

Nu wil ik de veiligheid maximaliseren. De class ondersteund https en http, de bestandsnaam wordt gecontroleerd op illegale tekens middels een regex. Het mimetype, de extensie en de bestandsgrootte worden gecontroleerd. Locale (chmod 0774) schrijfrechten worden maar een kort moment toegepast op de upload map, enkel tijdens het copieren van een bestand van zijn tijdelijke plaats op de server naar zijn uiteindelijke locatie waarna weer leesrechten worden toegepast (chmod 700).
De upload class werkt met sessies. Dus het object wordt in de sessie geplaatst en moet met een pointer aangeroepen worden na 1 keer aangemaakt te zijn binnen de sessie. Buiten een sessie om kan de class niet gebruikt worden.

Zijn er nog meer dingen die kan doen om de veiligheid te verhogen? Ik weet dat controle op mime-type niet waterdicht is. Zijn er nog methoden die ik kan toepassen ter controle die zekerder maken dat een bestand wat geupload wordt ook daadwerkelijk is hoe het zich voordoet?

[voodooless]

Check file headers op de juistheid van file type.

[Spider.007]

Als je de webserver een chmod 774 laat uitvoeren vanaf een 700 directory; dan is deze waarschijnlijk ge-owned door de webserver. Dan hoef je die directory toch niet te chmod-en om te mogen schrijven? Dit lijkt me een leuk gevoel; maar nutteloos stuk veiligheid?

Als je nuttige input wil moet je gewoon de code hier posten; dan krijg je vanzelf heel erg veel commentaar

voodooless schreef op vrijdag 23 november 2007 @ 13:41:
Check file headers op de juistheid van file type.

Dat lijkt me niet slim; dat is namelijk slechts wat de browser doorstuurt; en die kan je uiteraard niet vertrouwen Beter is het te kijken naar http://nl2.php.net/finfo_open

edit:
even over het woord file heengelezen

[ Voor 3% gewijzigd door Spider.007 op 24-11-2007 15:25 ]

[cspare]

Ik ben niet dat je zegt een guru op het gebied van linux rechten:P maar wat ik zou verwachten is dat chmod 774 ook execute rechten geeft, en dat lijkt me niet nodig en dus onveilig. daarnaast chmod 700 geeft de owner meer dan alleen leesrechten, misschien handig om ook een chown te doen dan, naar een andere user.

Ik vermoed trouwens dat voodooless niet de HTTP headers bedoeld, maar de file headers (zoals hij ook letterlijk zegt:x) en dus eigenlijk hetzelfde bedoeld als Spider.007, right?

[kokx]

Ik raad je aan om de inhoud en het type uit de HTTP headers te gaan vergelijken. Aangezien iedereen die een beetje handig is met sockets het type daarvan kan veranderen.

Ik gebruik voor plaatjes meestal een check als deze:

PHP:

<?php
switch ($_FILES['namefromform']['type']) {
    case 'image/png':
    case 'image/x-png':
        $img = @imagecreatefrompng($_FILES['namefromform']['tmp_name']);
        break;
    case 'image/jpeg':
    case 'image/pjpeg':
    case 'image/jpg':
        $img = @imagecreatefromjpeg($_FILES['namefromform']['tmp_name']);
        break;
    case 'image/gif':
        $img = @imagecreatefromgif($_FILES['namefromform']['tmp_name']);
        break;
    default:
        $img = false;
}

if (!$img) {
    // de image is ongeldig, geef een error
} else {
    imagedestroy($img);
}

[djiwie]

cspare schreef op zaterdag 24 november 2007 @ 13:28:
Ik ben niet dat je zegt een guru op het gebied van linux rechten:P maar wat ik zou verwachten is dat chmod 774 ook execute rechten geeft, en dat lijkt me niet nodig en dus onveilig.

Je hebt op een map execute rechten nodig om te kunnen bladeren in een map, het is dus niet van invloed op de wel-of-niet uitvoerbaarheid van bestanden in een map (die moet je per bestand executable maken).

[Bram77]

Bedakt voor de nuttige tips! Hier kan ik zeker wat mee.

[FragFrog]

Bram77 schreef op vrijdag 23 november 2007 @ 13:18:
PHP4 (750 regels code zonder comments)

Ben ik de nou echt de enige die bij deze regel al een idee krijgt van wat er beter kan?

Serieus, als jij 750 regels PHP code nodig hebt voor een file-upload class vraag ik me ten zeerste af wat je er allemaal ingezet hebt; dat je aan het developpen bent voor een taal die over een paar weken officieel niet meer ondersteunt wordt is zo mogelijk nog verontrustender

[Bram77]

Tsja, beter kan het zeker. Ik heb nog nooit terruggekeken naar code waarbij ik niet het idee had dat het beter kon. Maar of 750 regels code veel is valt niet te beoordelen totdat je de code hebt gezien. Dus je opmerking over "wat er beter kan" is vrij loos. Het zegt meer over jouw perceptie dan over mijn code.

De migratie naar PHP5 is een 'work in progress'. In de huidige situatie betekend dat namelijk dat ik alles wat met testen en aanpassen van code te maken heeft in eigen tijd doe. Waarmee ik wil zeggen dat je de daadwerkelijke situatie in zijn geheel niet kent en dus onmogelijk een oordeel kunt vellen.
Als je het mij vraagt is je volledige post vrij overbodig en getuigd het van arrogantie.

[ Voor 21% gewijzigd door Bram77 op 25-11-2007 01:19 ]

[FragFrog]

Mijn excuses, die opmerking over 'wat er beter kon' sloeg vooral op de woorden 'zonder comments' - zeker gezien je ondertitel. Ik ging er vanuit dat die smiley erachter wel duidelijk zou maken dat het niet serieus bedoelt was

Wat betrefd de rest: ik heb vrij veel ranzige code gezien (heb een paar collega's waarvan ik liever code herschrijf dan implementeer), vaak in enorme classes die, wanneer je er eens kritisch naar gaat kijken, vaak in minder dan de helft kunnen. Ik vraag me serieus af hoe je 750 regels code kan schrijven voor iets als file-uploads, tenzij je daar complete HTML pagina's in propt zou ik namelijk niet weten hoe je daar meer dan 300 ~ 400 regels aan kwijt kan zijn (de laatste fileupload library die ik zelf geschreven heb doet wat standaard zaken als renamen, verplaatsen, headers controleren en rechten toezetten in pak'm beet 70 regels). Het klonk wellicht sarcastisch, maar ik ben oprecht benieuwd naar wat je er dan allemaal inzet