Advanced PHP, maar dan anders

Door EdwinG op zondag 10 oktober 2010 13:01 - Reacties (14)
Categorie: -, Views: 4.132

Via dit Topic op het forum kwam ik de volgende blogpost tegen:

7 useful functions to tighten security

Met een 'prachtige' lijst van 7 functies die de beveiliging van een PHP-script kunnen verbeteren. Daarnaast veel manieren hoe het niet moet. Helaas vermelden ze dat laatste er zelf niet bij.

1) mysql_real_escape_string() - This function is very useful for preventing from SQL Injection Attack in PHP . This function adds backslashes to the special characters like quote , double quote , backslashes to make sure that the user supplied input are sanitized before using it to query. But, make sure that you are connected to the database to use this function.
Ok, handig om te vermelden dat je een verbinding met de database moet opzetten, voordat je deze functie gebruikt. De functie houdt immers rekening met de karakterset van de verbinding. Maar als de tutorial zo 'advanced' is, zou je toch ook verwachten dat het gebruik van prepared statements, al dan niet via PDO wordt omschreven. Een gemiste kans, en dat al in de eerste paragraaf.
2) addslashes() – This function works similar as mysql_real_escape_string(). But make sure that you don’t use this function when “magic_quotes_gpc” is “on” in php.ini. When “magic_quotes_gpc” is on in php.ini then single quote(‘) and double quotes (“) are escaped with trailing backslashes in GET, POST and COOKIE variables. You can check it using the function “get_magic_quotes_gpc()” function available in PHP.
Wie nog gelooft in addslashes() moet dit artikel maar eens lezen.
En een trailing backslash? Is trailing niet achter het teken? Daar is een backslash vrij nutteloos.
Over magic_quotes hoef ik hopelijk niet te beginnen, daar is de php-website erg duidelijk over.
3) htmlentities() – This function is very useful for to sanitize the user inputted data. This function converts the special characters to their html entities. Such as, when the user enters the characters like “<” then it will be converted into it’s HTML entities < so that preventing from XSS and SQL injection attack.
Dus....
htmlentities() gebruiken om SQL-injectie tegen te gaan. Dit is niet alleen dom, maar ronduit gevaarlijk. Standaard wordt het enkele aanhalingsteken (') in ieder geval niet gecodeerd. En de backslash sowieso niet. Hoe makkelijk wil je het maken voor scriptkiddies?
4) strip_tags() – This function removes all the HTML, JavaScript and php tag from the string. But you can also allow particular tags to be entered by user using the second parameter of this function.
Hier mis ik een paar waarschuwingen. Zeker als het om beveiliging gaat, is het toch wel handig om te beseffen dat strip_tags() niets veranderd aan de volgende gebruikersinvoer:

code:
1
x" onMouseOver="alert(document.cookie);" class="


Mocht je gebruikersinvoer door strip_tags() halen, en vervolgens in een invoerveld weer tonen, dan is de applicatie nog steeds vatbaar voor Cross-Site Scriptingaanvallen.

code:
1
<input type="text" name="username" value="{USER_INPUT}" />

5) md5() – Some developers store plain password in the database which is not good for security point of view. This function generates md5 hash of 32 characters of the supplied string. The hash generated from md5() is not reversible i.e can’t be converted to the original string.
Ok, dat het plain-tekst opslaan van wachtwoorden niet verstandig is spreekt voor zich. Op dat punt dus een goed stukje...
Maar, er staat nog steeds 'advanced' in de titel van de blog. En dan denk ik toch dat het wel eens handig kan zijn om te vermelden dat:
  • md5 een vrij zwak hashalgoritme is, en beter vermeden kan worden.
  • Salts een toegevoegde waarde hebben
  • Key-strengthening misschien wel overwogen moet worden
  • Er een veel algemenere Hashfunctie bestaat
Nr 6 (sha1) sla ik over, daar geldt hetzelfde voor als punt 5.
7) intval() – Please don’t laugh. I know this is not a security function, it is function which gets the integer value from the variable. But you can use this function to secure your php coding. Well, most the values supplied in GET method in URL are the id from the database and if you’re sure that the supplied value must be integer then you can use this function to secure your code.

$sql=”SELECT * FROM product WHERE id=”.intval($_GET['id']);

As, you can see above, if you’re sure that the input value is integer you can use intval() as a secrity function as well.
Als ik zeker ben dat iets een integer is, kan ik intval() gebruiken? Dat is dus precies verkeerd om. Als ik intval() gebruik, weet ik daarna zeker dat ik met een integer te maken heb. Mocht de invoer niet om te zetten zijn naar een integer, dan heb je daarna een 0. En het voorbeeld is op zich ook weer gevaarlijk. Genoeg lezers zullen het querievoorbeeld zo overnemen, en op moment dat ze niet met een integer te maken hebben intval() weg laten, waardoor SQL-injectie weer mogelijk wordt.


Tot zover de zeven 'goede' functies, en veel meer slecht advies. Trap er niet in.

Volgende: Valideren kun je leren 06-'09 Valideren kun je leren

Reacties


Door Tweakers user Spesh, zondag 10 oktober 2010 13:09

Nvm, goede tips. Eigenlijk zou er juist een stap voor stap tutorial moeten komen hoe je SQL injections uitvoert zodat ontwikkelaars hun eigen werk kunnen testen, de meeste kijken hoe je sql injections kan tegengaan en nemen het over zonder te testen of het ook echt werk.

[Reactie gewijzigd op zondag 10 oktober 2010 13:18]


Door Tweakers user wheez50, zondag 10 oktober 2010 13:20

Zeer goede 7 artikelen. Vooral omdat er mensen zoals jij goed commentaar op kunnen leveren. Dank je voor de toevoegingen :)

Door Tweakers user i-chat, zondag 10 oktober 2010 14:13

@Spesh,
Eigenlijk zou er juist een stap voor stap tutorial moeten komen hoe je SQL injections uitvoert zodat ontwikkelaars hun eigen werk kunnen testen,
kun je dan niet beter een soort tooltje maken (een firefox extentie bijvoorbeeld) die dat voor je doet,
zodat je een soort 'rapport' krijgt van de mogelijke zwakten die er zijn ontdekt.

Door Tweakers user Spesh, zondag 10 oktober 2010 14:17

i-chat schreef op zondag 10 oktober 2010 @ 14:13:
@Spesh,
[...]
kun je dan niet beter een soort tooltje maken (een firefox extentie bijvoorbeeld) die dat voor je doet,
zodat je een soort 'rapport' krijgt van de mogelijke zwakten die er zijn ontdekt.
Dat zou geen slecht idee zijn, geen idee of zoiets al bestaat.

Door Tweakers user EdwinG, zondag 10 oktober 2010 14:33

Spesh schreef op zondag 10 oktober 2010 @ 14:17:Dat zou geen slecht idee zijn, geen idee of zoiets al bestaat.
Er bestaan diverse tools de een website kunnen controleren op sql-injectie, al dan niet commercieel. Bijvoorbeeld SQLmap (gratis), Burp suite en Acunetix web vulnerability scanner.

SQLmap
Aansturing is, in de makkelijkste manier, het aanpassen van het configuratiebestand, en dan het programma starten (CLI) met als parameter het aangepaste configuratiebestand. Geeft niet alleen aan of iets vatbaar is, maar kan ook nog een hoop gegevens uit de database halen, wat de ernst van zo'n beveiligingslek kan aanduiden. De aanvallen zijn niet bepaald 'low-profile'.

Burp suite
Intercepting proxy, maar kan ook actief spideren & scannen. Geeft meer aan dan SQL, maar mist ook een hoop.

Door Tweakers user Spesh, zondag 10 oktober 2010 15:04

Heb SQLmap even bekeken en dat ziet er alvast veelbelovend uit, bedankt voor de tip!

Door Tweakers user mzziol, zondag 10 oktober 2010 17:46

Gewoon niet direct SQL aanroepen, maar gebruik maken van een MCV?

Door Tweakers user EdwinG, zondag 10 oktober 2010 17:59

mzziol schreef op zondag 10 oktober 2010 @ 17:46:
Gewoon niet direct SQL aanroepen, maar gebruik maken van een MCV?
Bedoel je hier een Model-View-Controller mee? Het MVC-patroon bied uit zichzelf geen bescherming tegen SQL-injectie.

Door Tweakers user analog_, zondag 10 oktober 2010 18:24

*sigh*, al die checks duw je in een GET/POST object dat het automagisch sanitized. Eigenlijk komt het altijd neer op het volgende: vertrouw nooit je input, check tot je 100% kan garanderen dat het geen exceptions opgooid. Ook dropdowns en dergelijke moet je checken aangezien iemand een aangepaste POST/GET kan versturen.
EdwinG schreef op zondag 10 oktober 2010 @ 17:59:
[...]

Bedoel je hier een Model-View-Controller mee? Het MVC-patroon bied uit zichzelf geen bescherming tegen SQL-injectie.
It does, aangezien je in je View moet sanitizen om je controller niet de mist in te laten draaien. Dit werkt natuurlijk gemakkelijker in strong typed languages terwijl in PHP het 'toevallig' werkt (bv. integer in string als argument van een controller methode).

[Reactie gewijzigd op zondag 10 oktober 2010 18:27]


Door Tweakers user EdwinG, zondag 10 oktober 2010 19:43

analog_ schreef op zondag 10 oktober 2010 @ 18:24:
It does, aangezien je in je View moet sanitizen om je controller niet de mist in te laten draaien. Dit werkt natuurlijk gemakkelijker in strong typed languages terwijl in PHP het 'toevallig' werkt (bv. integer in string als argument van een controller methode).
Dan moet je dus zelf in je view not de validatie/codering uitvoeren. Het patroon helpt dus niet, maar de manier waarop je met je view omgaat.

Door Tweakers user YopY, maandag 11 oktober 2010 09:14

Ik vindt het grappig dat na 10 jaar dit soort 'adviezen' nog steeds gegeven worden (en met 'grappig' bedoel ik 'bedroevend'). Zend zelf zou hier ook iets tegen moeten doen, bijvoorbeeld door met grote letters boven de mysql_real_escape_string() en addslashes() functies neer te zetten dat het geen beveiligingsfuncties zijn en dat de gebruiker ten allen tijde prepared statements moet gebruiken.

Ook bij de andere aangehaalde functies moet bij staan dat die niet beschermen tegen bijvoorbeeld XSS.`

Edit: even een opmerking erbij gezet voor zowel de originele auteur als zijn evt. lezers. Ik zou graag willen zien dat één van de eerste opmerkingen bij dat soort posts een grote 'NEE DOE DIT NIET!11' is.

[Reactie gewijzigd op maandag 11 oktober 2010 09:27]


Door Tweakers user ajakkes, maandag 11 oktober 2010 10:02

Wat ik toch een beetje mis in dit verhaal is het:
Maar doe het dan zo gehalte.

Door Tweakers user -RetroX-, maandag 11 oktober 2010 10:06

En dan de benaming: "Advanced PHP".... als het conceptueel al niet deugt heeft het geen zin om aan code te beginnen.... ongeacht de taal. Ik blijf het jammer vinden dat veel tutorialschrijvers toch zulke slechte voorbeelden gebruiken.

Het wordt tijd dat PDO simpelweg de standaard wordt voor databasegebruik. Ben je meteen van dit gelazer af.
(vanaf PHP6 wordt PDO standaard en komen de specifieke mysql_ en pgsql_ etc te vervallen, helaas zal dat nog wel enkele maanden (of jaren) duren.)

Door Tweakers user EdwinG, maandag 11 oktober 2010 17:29

YopY schreef op maandag 11 oktober 2010 @ 09:14:
Zend zelf zou hier ook iets tegen moeten doen, bijvoorbeeld door met grote letters boven de mysql_real_escape_string() en addslashes() functies neer te zetten dat het geen beveiligingsfuncties zijn en dat de gebruiker ten allen tijde prepared statements moet gebruiken.
Dat klopt niet helemaal, mysql_real_escape_string() is wel degelijk een beveiligingsfunctie. Het probleem met de functie is dat het een lapmiddel is voor een fundamenteel verkeerde aanpak, namelijk het mengen van code en data.
YopY schreef op maandag 11 oktober 2010 @ 09:14:
Ook bij de andere aangehaalde functies moet bij staan dat die niet beschermen tegen bijvoorbeeld XSS.`
htmlentities, indien correct gebruikt, helpen wel degelijk tegen XSS. De functie correct gebruiken is daar wel noodzakelijk voor, net als weten waneer de functie niet meer toereikend is, en andere maatregelen noodzakelijk zijn.
YopY schreef op maandag 11 oktober 2010 @ 09:14:even een opmerking erbij gezet voor zowel de originele auteur als zijn evt. lezers. Ik zou graag willen zien dat één van de eerste opmerkingen bij dat soort posts een grote 'NEE DOE DIT NIET!11' is.
Bedoel je bij de bron van mijn artikel? Daar zie ik op dit moment slechts 1 reactie staan, en dat is een reactie over filter_var. Moest je reactie eerst nog goedgekeurd worden, of is deze verwijderd door de auteur van de blog?
ajakkes schreef op maandag 11 oktober 2010 @ 10:02:
Wat ik toch een beetje mis in dit verhaal is het:
Maar doe het dan zo gehalte.
Had ik misschien inderdaad duidelijker kunnen maken. Nu geef ik wel steeds aan wat er mis is, of wat ik mis, en daarmee impliciet een beginpunt voor hoe het wel moet.
Bijvoorbeeld het advies om prepared statements te gebruiken, of salts & key strengthening toe te passen.
-RetroX- schreef op maandag 11 oktober 2010 @ 10:06:
Het wordt tijd dat PDO simpelweg de standaard wordt voor databasegebruik. Ben je meteen van dit gelazer af.
(vanaf PHP6 wordt PDO standaard en komen de specifieke mysql_ en pgsql_ etc te vervallen, helaas zal dat nog wel enkele maanden (of jaren) duren.)
Ik denk zelfs dat het veel langer gaat duren. Het zal maanden (of jaren?) duren voordat PHP6 uit wordt gebracht. Helaas wordt op dit moment nog steeds PHP4 gebruikt. Zeker door het verdwijnen van de mysql_-functies zal de overstap van PHP5 naar PHP6 nog langer gaan duren.
En voor de sites die nu nog op PHP4 draaien: van 4 naar 6 lijkt me onmogelijk zonder de code te herschrijven.

Reageren is niet meer mogelijk