[php] Veilig zoeken in database via $_GET? *

[BetuweKees]

hi,

ben momenteel bezig met een dynamische site waarop ik doormiddel van oa mod_rewrite mooi, statische links kan aanmaken (oa voor duidelijkere tracking, en gebruiks en zoekmachine vriendelijkheid). heb dit al eens gedaan, maar dan puur door het weergeven van verschillende niveaus doormiddel van integeers (bv een link als /fotoboek/9/1/ herschrijven naar 'laat foto 1 uit boek 9 zien)
aangezien een tekstuele link imho duidelijker is, ben ik momenteel bezig iets dergelijks te implementeren, maar ik maak me een beetje zorgen over of een en ander wel veilig in elkaar zit (niet echt zin in een of andere hacker die doormiddel van het aanpassen van een url me database weet leeg te halen)

wat ik momenteel doe is:

.htaccess

code:

RewriteRule   ^verslag/(.*)/$      verslagen.php?verslag=$1

verslagen.php

PHP:

// Check of url wel via mod_rewrite gekomen is, anders 403
if(strstr($_SERVER['REQUEST_URI'], ".php") != false) { do_403(); }

// Kijk of variabelen opgegeven zijn, anders 404
if(empty($_GET['verslag'])) { do_404(); }
if(!preg_match("/^[a-z0-9\-_]/", $_GET['verslag'])) { do_404(); }

// Met SQL verslag opzoeken
[..]
$sqlQuery .= "WHERE trackback = '". $_GET['verslag'] ."' ";

mijn vraag: is dit veilig, of zijn er nog steeds manieren waarop iemand na al deze checks rotzooi kan trappen in mijn database?

edit:

oeps, vergeten titel af te maken..
mag ik een titelfix naar "[php] Veilig zoeken in database via $_GET?"

[ Voor 9% gewijzigd door BetuweKees op 28-12-2004 21:28 . Reden: titel vergeten :( ]

[Verwijderd]

Dollartekentje aan het einde van je regexp?

Voor de rest ben je redelijk paranoide. Wat maakt het uit als iemand de pagina direct aanroept. Als de variabele maar geen troep bevat. Ik doe het meestal alleen met integers, wordt het nog makkelijker:

$verslag=(int)$_GET[verslag];

Toch?

Wacht, die regexp moet volgens mij zo zijn:

preg_match("/^[a-z0-9\-_]+$/", $_GET['verslag'])

(plusje en dollarteken, anders check je alleen het eerste karakter)

[ Voor 23% gewijzigd door Verwijderd op 28-12-2004 21:34 ]

[kvdveer]

In je regexp ontbreekt nog een einde-anker, "$", maar zelfs nu is je code al redelijk onkraakbaar. Het is ook vrij eenvoudige code - dat helpt...

Als je server register_globals en safe_mode aan heeft staan, dan ben je nog veiliger...

[BetuweKees]

nooit gedacht dat ik het positief zou vinden als iemand me paranoide noemt
ook even eind anker toegevoegd, was ik inderdaad vergeten (slordig..), maar nu kan ik dus rustig gaan slapen..

[nxt]

Verder kun je nog de functie 'mysql_escape_string' (ervanuitgaande dat je mysql gebruikt als database) gebruiken. Hierdoor voorkom je dat gebruikers nare dingen die je query veranderen invullen.

code:

$sqlQuery .= "WHERE trackback = '".mysql_escape_string( $_GET['verslag']) ."' ";

[JasperE]

nxt schreef op woensdag 29 december 2004 @ 01:50:
Verder kun je nog de functie 'mysql_escape_string' (ervanuitgaande dat je mysql gebruikt als database) gebruiken. Hierdoor voorkom je dat gebruikers nare dingen die je query veranderen invullen.

code:

$sqlQuery .= "WHERE trackback = '".mysql_escape_string( $_GET['verslag']) ."' ";

Dat zou ik zeker doen ja anders is je script alles behalve veilig, tijd om snel weer ff wakker te worden dus

[tombo_inc]

hij heeeft zijn string toch al tussen quotes staan? of snap ik er dan niks van.

[PowerSp00n]

nxt schreef op woensdag 29 december 2004 @ 01:50:
Verder kun je nog de functie 'mysql_escape_string' (ervanuitgaande dat je mysql gebruikt als database) gebruiken. Hierdoor voorkom je dat gebruikers nare dingen die je query veranderen invullen.

code:

$sqlQuery .= "WHERE trackback = '".mysql_escape_string( $_GET['verslag']) ."' ";

In dit geval wat overbodig, er kunnen namelijk geen tekens in voorkomen (regex) die gescaped zouden moeten worden. Verder doe ik het ook met regex'es, alles wat via POST/GET en cookies binnenkomt is bij mij voorspelbaar en dus gewoon overal een regex overheen. Verder moet je volgens de docs tegenwoordig mysql_real_escape_string gebruiken (wie verzint die lange namen pff ).

[Verwijderd]

BetuweKees schreef op woensdag 29 december 2004 @ 01:04:
nooit gedacht dat ik het positief zou vinden als iemand me paranoide noemt
ook even eind anker toegevoegd, was ik inderdaad vergeten (slordig..), maar nu kan ik dus rustig gaan slapen..

Gedeeltelijk is het inderdaad goed om paranoide te zijn maar als je zo'n lap code schrijft die net zo goed (en veilig) te vervangen zou zijn met...

$verslag= (int) $_GET['verslag'];

(je gebruikte toch alleen integers?)

...ben je toch niet helemaal optimaal bezig n.m.m. Je code wordt er nl. onoverzichtelijker door en er is meer kans op fouten, zoals hier de vrij ernstige fout in je regexp. Een van de belangrijkste regels in de automatisering is niet voor niets KISS.

Keep It Simple, Stupid!

[Verwijderd]

Wellicht om dit in een include bestandje te zetten?

foreach($_GET as $key=>$value) {
//validatie code hier
}

zelfde geldt voor $_POST, etc. Als je dit dan in iedere pagina include weet je zeker dat voortaan alle request vars ok zijn (tenzij je je check verkeerd doet ) en hoef je er niet meer naar om te kijken.

[Verwijderd]

kvdveer schreef op dinsdag 28 december 2004 @ 23:37:
In je regexp ontbreekt nog een einde-anker, "$", maar zelfs nu is je code al redelijk onkraakbaar. Het is ook vrij eenvoudige code - dat helpt...

Als je server register_globals en safe_mode aan heeft staan, dan ben je nog veiliger...

Een typfoutje hoop ik? register_globals juist uit Maar dit heeft op zich weinig met veiligheid te maken als je gewoon altijd $_GET, $_SESSION, etc gebruikt, of register_globals nou aan of uit staat..

En safe_mode aan vind ik persoonlijk eigenlijk zwaar kut *kuch* lichtelijk irritant. Levert alleen maar minder mogelijkheden op voor de programmeur zelf, wordt toch aangezet door hostingbedrijven om te zorgen dat de klanten hun servers niet te veel kunnen misbruiken / dingen te beperken?

[Verwijderd]

Verwijderd schreef op woensdag 29 december 2004 @ 11:43:
Een typfoutje hoop ik? register_globals juist uit Maar dit heeft op zich weinig met veiligheid te maken als je gewoon altijd $_GET, $_SESSION, etc gebruikt, of register_globals nou aan of uit staat..

Maakt wel degelijk uit. Ook al gebruik je altijd netjes $_GET etc, moet je nog erg uitkijken dat je geen ongeinitialiseerde waarden in je script gebruikt. Bijv:

PHP:

if ($condition) {
    $vlaggetje=1;
}

Als iemand je pagina aanroept met ?vlaggetje=1 erachter kan hij mogelijk (afhankelijk van de rest van je code) gekke dingen uithalen. Daarom is het altijd verstandig om je variabelen te initialiseren, bijv met '$vlaggetje=0' in het bovenstaande voorbeeld, of '$lijst=Array()'.

En safe_mode aan vind ik persoonlijk eigenlijk zwaar kut *kuch* lichtelijk irritant. Levert alleen maar minder mogelijkheden op voor de programmeur zelf, wordt toch aangezet door hostingbedrijven om te zorgen dat de klanten hun servers niet te veel kunnen misbruiken / dingen te beperken?

Kut, ja... Maar bedenk wel het volgende: Als safe mode niet aan staat draaien alle scripts onder de zelfde 'user' (meestal 'apache' of 'nobody') met dezelfde rechten. Dat betekent dat ieder php script op een server alles (inclusief scripts) kan lezen wat alle andere scripts kunnen lezen. Het is dus onmogelijk om bijv. database wachtwoorden van andere servergebruikers af te schermen. M.a.w. iedereen die bij je op de server zit kan bij je database. Safe mode is dus om jezelf te beschermen tegen de andere gebruikers. Die hostingbedrijven zelf maakt het niet veel uit, die zorgen wel dat 'apache' en 'nobody' niet bij de belangrijke systeembestanden kunnen.

De enige remedie is om je eigen colo server te nemen. Stuk relaxter, hoef je niet eens een wachtwoord op je DB te zetten

[ Voor 7% gewijzigd door Verwijderd op 29-12-2004 12:11 ]

[BetuweKees]

Verwijderd schreef op woensdag 29 december 2004 @ 10:55:
[...]

Gedeeltelijk is het inderdaad goed om paranoide te zijn maar als je zo'n lap code schrijft die net zo goed (en veilig) te vervangen zou zijn met...

$verslag= (int) $_GET['verslag'];

(je gebruikte toch alleen integers?)

nee niet dus..

in TS schreef ik dat ik in het verleden soortgelijke dingen gedaan heb, maar dan altijd met ints die inderdaad lekker makkelijk af te schremen zijn (kan je zelfs al in rewrite opnemen dat het een int is, dus is zelfs die extra check min of meer overbodig)

nu is het echter zo dat ik met een nieuw project bezig ben, waarbij ik strings wil gebruiken, aangezien de url's daar nog duidelijker van worden (en ook omdat ik een soort verslagen database wil maken, kan ik de titel van het verslag dus in de url opnemen)

nogmaals bedankt voor de tips

[Verwijderd]

BetuweKees schreef op woensdag 29 december 2004 @ 14:55:
nee niet dus..

in TS schreef ik dat ik in het verleden soortgelijke dingen gedaan heb, maar dan altijd met ints die inderdaad lekker makkelijk af te schremen zijn (kan je zelfs al in rewrite opnemen dat het een int is, dus is zelfs die extra check min of meer overbodig)

Als je ergens nou *niet* je beveiliging moet onderbrengen is het wel in je rewrite ;-)

nu is het echter zo dat ik met een nieuw project bezig ben, waarbij ik strings wil gebruiken, aangezien de url's daar nog duidelijker van worden (en ook omdat ik een soort verslagen database wil maken, kan ik de titel van het verslag dus in de url opnemen)

Ik snap het. Duidelijke URL's zijn i.d.d. ook een overweging.

nogmaals bedankt voor de tips

Graag gedaan.