Veilig?
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
2
3
4
5
6
7
8
9
10
11
12
13
14
15
Het is de bedoelding als je $_GET['p'] hebt dat ie dan de pagina include.
Alvast bedankt!
Jouw manier kan sowieso korter:
Code (php)
Veiliger is het om gebruik te maken van de switch constructie www.php.net/switch of om de paginaverwijzingen op te slaan in een database. Voorbeeldje van switch:
Code (php)
Gewijzigd op 21/04/2011 12:35:07 door Ozzie PHP
Hoezo is die switch veiliger? want dan kan ik via de adres balk toch overal komen?
Alleen pagina's die in de switch zijn opgenomen als 'case' kunnen worden aangeroepen.
Ozzie:
Veiliger is het om gebruik te maken van de switch constructie
Bob:
Hoezo is die switch veiliger? want dan kan ik via de adres balk toch overal komen?
Terechte opmerking. Het is niet veiliger. Het is enkel een andere manier van werken, waardoor het makkelijker is veiliger te werken. Echter zou ik in dit geval toch opteren voor een if-else structuur. Die structuur is krachtiger en daardoor kan je het iets flexibeler maken. Je hoeft nl. niet elke case te definiëren, bespaart je tijd. Een case-switch zal echter wel iets sneller doorlopen worden dan een if-else. Echter moet hier maar 1 statement gecontroleerd worden, dus maakt het sowieso amper uit. Eveneens is het zo dat je veel meer cases nodig hebt om hetgeen te bereiken wat ik in onderstaand script heb geplaatst. Dus, je komt sneller uit met een if-else ;-)
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
<?php
$sPage = $_GET['p'];
$aDissalowed = array('admin.php', 'payments.php', 'secret.php');
/*
* In dit geval is file_exists meer toepasselijk dan is_file
* In je voorbeeld waarin je een directory uitleest, zou is_file een betere keuze zijn
*/
if(file_exists('/path'.$sPage) && !in_array($sPage, $aDissalowed)) {
/*
* Het bestand bestaat -> OK
* Het bestand mag bekeken worden -> OK
*/
include_once('/path/'.$sPage);
}else {
/*
* Je komt in else structuur omdat:
* - Bestand niet bestaat
* - Bestand bekijken niet toegestaan is
*/
include_once('/path/home.php');
}
/* Dit is veilig + flexibeler dan Ozzie's methode */
?>
$sPage = $_GET['p'];
$aDissalowed = array('admin.php', 'payments.php', 'secret.php');
/*
* In dit geval is file_exists meer toepasselijk dan is_file
* In je voorbeeld waarin je een directory uitleest, zou is_file een betere keuze zijn
*/
if(file_exists('/path'.$sPage) && !in_array($sPage, $aDissalowed)) {
/*
* Het bestand bestaat -> OK
* Het bestand mag bekeken worden -> OK
*/
include_once('/path/'.$sPage);
}else {
/*
* Je komt in else structuur omdat:
* - Bestand niet bestaat
* - Bestand bekijken niet toegestaan is
*/
include_once('/path/home.php');
}
/* Dit is veilig + flexibeler dan Ozzie's methode */
?>
Gewijzigd op 21/04/2011 13:07:06 door Write Down
Wat denk je van ?p=../path/admin.php
Dan kan ik alsnog het bestand admin.php includen. (En verder alle andere bestanden waar de gebruiker bij kan waar de website onder draait)
Oftewel het is NIET veilig.
Gewijzigd op 21/04/2011 13:10:33 door Write Down
Foutje? manier om je hele server op en bloot te geven aan een hacker.
TJVB tvb op 21/04/2011 13:11:50:
Foutje? manier om je hele server op en bloot te geven aan een hacker.
Heb je gelijk in. grote fout
Gewijzigd op 21/04/2011 13:13:30 door Write Down
Maar bedankt in ieder geval dat er naar gekeken is. Top
Bob, jouw manier zorgt er dus voor dat je iedere willekeure PHP bestand kan includen. Hierin kun je dus geen white/black list maken. Niet veilig dus.
Write Down op 21/04/2011 12:53:29:
Terechte opmerking. Het is niet veiliger.
Ozzie:
Veiliger is het om gebruik te maken van de switch constructie
Bob:
Hoezo is die switch veiliger? want dan kan ik via de adres balk toch overal komen?
Terechte opmerking. Het is niet veiliger.
Het is wel degelijk veiliger. Je kunt op deze manier helemaal niet via de adresbalk overal komen. Lees anders de documentatie van 'switch' even door zodat je het principe leer begrijpen.
Waar het in ieder geval op neer komt is dat je moet voorkomen dat iemand klakkeloos pagina's kan aanroepen. Je moet van tevoren definiëren welke pagina's mogen worden aangeroepen.
Dit is ook een optie:
Code (php)
Gewijzigd op 21/04/2011 13:44:28 door Ozzie PHP
@Bob, het grootste nadeel bij jouw methode is dat elke keer de map doorlopen wordt. Dit kan een zwaar proces zijn.
Ik weet wat de switch-case inhoudt. Ik blijf er echter bij dat een if-else structuur krachtiger is. Op de manier dat jij het gebruikt uiteraard niet, dan ben je beter af met switch-case. Op de manier dat ik het doe, is het flexibeler. (mits je nog even mijn fout verbeterd)
Bedankt voor het topic, ik heb weer wat beveiliging uit te voeren.
Tobias Witmer op 21/04/2011 15:34:00:
Je kunt de array $allowed_pages van Ozzie toch ook vullen vanuit je database? dan hoef je je broncode niet steeds aan te passen als je je website uitbreidt.
Bedankt voor het topic, ik heb weer wat beveiliging uit te voeren.
Bedankt voor het topic, ik heb weer wat beveiliging uit te voeren.
Precies! Je krijgt dan het principe van een router en je bent flexibel met het instellen van je urls. Meteen even wat htaccess rewrite toepassen en SEO technisch kun je er ook weer tegen :)
Kan je niet ook met preg_match kijken of er een punt inzit zoja naar homepage
Jordi kroon op 21/04/2011 17:05:41:
Kan je niet ook met preg_match kijken of er een punt inzit zoja naar homepage
Dat kan wel, maar waarvoor wil je dat gebruiken? voor de ../path?
Ja klopt inderdaad zwaar process maar al die pagina's staan achter een login systeem en zodra de sessie niet bestaat kan die pagina ook niet weergegeven worden. Dus vandaar dat ik er niet een allowed pages var inzet. Maar ik denk wel inderdaad dat ik het via een array de pagina's ga doen want dan hoeft ie inderdaad niet steeds de map te doorzoeken.
Je kunt het ook eenmalig doen en de gevonden gegevens in een session stoppen. Dan hoef je niet elke keer de array te updaten als je een nieuw bestand toevoegd. Ik zou wel kijken naar glob() dat is sneller dan dat wat je nu hebt.
var_export dat die array van pagina's genereert en de uitvoer in een PHP bestandje opslaan. Of je maakt dat automatisch zodra dat bestand mist. Voeg je een nieuw bestand toe? Dan delete je dat list-bestandje, en je script genereert hem automatisch opnieuw.
Ik denk niet dat glob sneller is dan scandir. glob moet per bestand ook nog kijken of het voldoet aan het argument dat je meegeeft (dat pattern) en zoekt ook in directories. scandir geeft gewoon een lijst van bestanden in die map; het is niet meer dan opendir + readdir. Simpeler kan bijna niet.
Ik zou hem niet in een sessie stoppen. Zodra je dan een bestandje toevoegt zullen "nieuwe" gebruikers die wel kunnen zien, maar oude gebruikers niet. Daarnaast, hoe vaak verandert die data? Je kan makkelijker even een scriptje maken met Ik denk niet dat glob sneller is dan scandir. glob moet per bestand ook nog kijken of het voldoet aan het argument dat je meegeeft (dat pattern) en zoekt ook in directories. scandir geeft gewoon een lijst van bestanden in die map; het is niet meer dan opendir + readdir. Simpeler kan bijna niet.