Gomez12 schreef op woensdag 31 juli 2013 @ 12:13:
Idd, compleet waar.
In plaats van simpelweg alle invoer goed checken en sanitizen heb je ook mensen die zeggen gebruik library x die doet dat voor dit ene specifieke gevalletje voor je, en de rest van de gevallen die kom je vanzelf wel tegen...
Nou doe je alsof je input sanitizen een toverstaf is die je over je data heen haalt waarna alles veilig is. Niets is minder waar, en die denkwijze is erg gevaarlijk.
De problemen in kwestie ontstaan als externe input geinterpreteerd worden door software. Dus een string die geinterpreteerd wordt als SQL waardoor iemand de database kan manipuleren, als shell input waardoor iemand de shell meer kan laten doen dan de bedoeling is, als HTML waardoor XSS mogelijk wordt, etc, etc. Soms kun je niet anders dan escapen, maar in veel situaties kun je voorkomen dat de user input überhaupt ooit in het taaldomein in kwestie geinterpreteerd wordt (parametrized SQL queries zijn het canonieke voorbeeld).
Het probleem is dat wat je zou moeten doen om data te sanitizen afhankelijk is van het taaldomein waarin het geinterpreteerd wordt. Voor SQL moet je iets anders doen dan voor shell of HTML. Je kunt niet je input bij ontvangst sanitizen en er dan klaar mee zijn. Als je sanitized voor SQL dan heb je daar niks aan voor HTML en ook voor SQL is het niet afdoende (aangezien de quoting regels voor de bourne shell en SQL verschillen). Sanitizen voor HTML heb je niks aan voor SQL of shell. Sanitizen voor allemaal verneuk je je data mee. En als je toch wat verzint en de data moet later ergens heen waar je geen rekening mee gehouden had (json, een url, printf, whatever) dan is opgeslagen data niet sanitized en dus onveilig.
Dus, sanitizen kan eigenlijk alleen zinnig op het moment dat je de data voert aan een component/systeem dat die data interpreteert. Dus op het moment dat je het aan mysql_query() of exec() voert, of als onderdeel van HTML naar een client stuurt. En als je daar, op dat moment gaat sanitizen, waarom dan niet meteen een methode gebruiken die het interpreteren van de data helemaal voorkomt? Bij HTML ontkom je niet aan escapen, want HTML biedt geen methode om stukken data letterlijk te includen, maar voor SQL bieden parametrized queries precies dat, en voor exec() en vriendjes heb je vaak de mogelijkheid om de benodigde functionaliteit via een API te gebruiken in plaats van de command-line. En zo niet, dan kun je nog op zijn minst een functie gebruiken die niet een string aan een shell voert maar daadwerkelijk een lijst argumenten neemt en de binary in kwestie opstart met die argumenten, zoals POSIX' execl() (PHP biedt dit volgensmij met pcntl_exec()).
Als jij goed geschoonde parameters hebt dan maakt de library of exec-methode niet uit.
Dat is ook niet waar. Nasty tekens als ' of ; zijn geldige tekens voor een bestandsnaam en kunnen dus - afhankelijk van de situatie - in een commando voorkomen. Of de verschillende escape-functies afdoende zijn om dat af te vangen hangt er van af hoe de shell de zijn input precies interpreteert. En dat valt in dit geval helemaal niet te garanderen.
Om in het huidige voorbeeld te blijven, PHPs escapeshellarg() zet er single quotes omheen en behandelt verder alleen single quotes speciaal. Hoe een shell strings binnen single quotes interpreteert kan van shell tot shell verschillen. PHP's exec() voert /bin/sh uit (wat ik overigens nergens gedocumenteerd kan vinden...), maar dat kan nogal wat zijn. Op mijn systemen is dat typisch dash, op veel andere Linux systemen is het doorgaans bash in compatibility mode. Op 'echte' unices zoals Solaris kan het gerust de originele Bourne shell zijn. Het wordt interessanter als het csh is (niet ondenkbaar op oudere BSDs), die interpreteert namelijk ! binnen single quotes. Daar kun je waarschijnlijk weinig narigheid mee uithalen (tenzij shell history aanstaat, dan wordt het leuk), maar het toont goed aan hoe tricky "sanitizen" kan zijn.
Laat ik het zo zeggen : Jij moet geen internet pagina's gaan maken.
Haha, ik doe bijna niet anders de laatste tijd
Je moet javascript eruit filteren, je moet html-tags eruit filteren etc. etc. etc.
Nouja, ik verwoordde me in mijn vorige post wat ongelukkig; Ik bedoelde impliciet te vragen waarop ik de inhoud van een comment zogenaamd zou moeten controleren
voordat ik het opsla. XSS voorkomen is iets dat imho alleen zinnig is op het moment dat je het naar een client stuurt, en dat doe ik uiteraard ook (of nouja, dat doet het framework dat ik gebruik voor me, magoed).
Ik had het wat minder fel mogen verwoorden, maar: jep.
Input sanitizen is tricky business, en eigenlijk fundamenteel de verkeerde aanpak. Je input wordt geinterpreteerd, en dat is gevaarlijk. Dus wat doe je? Je input ombouwen zodat de interpretatie hopelijk veilig is. In plaats daarvan kun je beter voorkomen dat het geinterpreteerd wordt. Jouw voorstel escapeshellarg of escapeshellcmd te gebruiken maakt ook weer duidelijk hoe tricky escapen is. Voor escapeshellarg heb ik hierboven al een potentieel probleem aangegeven, en aan escapeshellcmd heb je al helemaal weinig. escapeshellcmd beschermt de code van de topicstarter bijvoorbeeld niet tegen de mogelijkheid arbitraire files te overschrijven:
code:
1
| { "imgrot": "5 -write /een/of/andere/file" } |
User input checken is nou niet bepaald vreemd
Maar checken is iets anders dan sanitizen. Sure, een rotatie-hoek is een getal, dus controleren of het daaraan voldoet is nuttig. Ik zei in mijn vorige post ook al dat het nuttig kan zijn om te kijken of input
zinnig is. Daarop vertrouwen voor security is wat anders, al zal dat bij een getal niet snel mis gaan. Bij strings wordt het een ander verhaal.
Heh, dat is escapen / sanitizen, en dat is nou juist waar ik tegen ageer in mijn posts
Niet omdat ik veilig met data omgaan stom vind, maar de manier waarop wel. Maar daar heb ik inmiddels wel genoeg woorden aan besteed.
Daarnaast kun je de command-line argumenten voor ImageMagick ook filteren/valideren/controleren/welke-term-je-dan-ook-wilt. Zo zal ImageMagick voor het "-rotate" argument altijd een getal verwachten, dus dan kun je iets eenvoudigs als is_int() of een cast toepassen.
Ja, voor een getal wel. Maar voor een tekst die ImageMagick op het plaatje moet zetten? Een bestandsnaam?
Zeggen dat "de enige fatsoenlijke manier" om ImageMagick aan te roepen via zo'n plugin is, is onzin.
Toch blijf ik daarbij. Ik heb hopelijk genoeg argumenten gegeven waarom het beter is user input niet in een shell-commando includen.
Los van de veiligheid zijn er trouwens meer redenen om Imagick te gebruiken in plaats van exec(). Als je meerdere operaties op een plaatje moet doen kun je dat met Imagick netjes doen in leesbare/onderhoudbare code met een lijst method-calls:
PHP:
1
2
3
| $img->rotate(...);
$img->crop(...);
img->annotate(...); |
Doe je het met exec(), dan kun je kiezen voor één exec per stap, wat inefficient is en je moet oppassen dat je temporaries uniek zijn zodat ze niet overschreven worden door andere threads. Of je kiest voor één exec, wat een behoorlijk onleesbare opdrachtregel op gaat leveren.
Of het feit dat je bij een fout met exec() een non-zero return code krijgt en de output string mag gaan lopen parsen of rechtstreeks aan de user doorgeven. Bij Imagick krijg je tenminste een exception bij de stap in kwestie, dat geeft je gestructureerdere informatie.
/u/289286/crop5f64c7d041185_cropped.png?f=community)
/u/107051/jeroenmadtrix60.png?f=community)
:strip_icc():strip_exif()/u/304716/enforcer_small.jpeg?f=community)
/u/26742/000000751.png?f=community)
:strip_exif()/u/2087/osiris_ani.gif?f=community)
:strip_exif()/u/138622/kikker.gif?f=community)
Thanks voor de reacties, inmiddels kunnen overleggen met de sys- admins waarom de module voor PHP toch (nodig) is.
