[php] mijn manier van paginas includen in index.php veilig?

[dossiewossie]

Ik wil dat index.php altijd wordt aangeroepen, en aan de hand van een get variabele of session variabele de juiste pagina wordt geinclude.

Er is een publiek gedeelte, daarnaast 4 verschillende gedeeltes voor 4 verschillende rollen.
Als iemand inlogt dan wordt $_SESSION['section'] geset. Dan wordt gekeken of er in de map met de naam $_SESSION['section'] een bestand aanwezig is met de naam $_GET['page'].

Ik heb gegoogled op manieren waarop ik dit kan doen, en wat de veiligheid is, maar ik mis even het overzicht of wat ik gemaakt heb echt veilig is. Moet ik hier bijvoorbeeld de user input converteren met htmlentities ofzoiets? Graag een voorbeeldje dan wat het probleem kan zijn.

De code van index.php :

PHP:

<?php
session_start();

// de root definieren
define("ROOT","D:/Mijn server/xampp/htdocs/private/kvk/");
// einde definieren root constant


function checkPage($sectie)
{
    $filename = $sectie .'/'. $_GET['page'];
    
    if (!ctype_alnum($_GET['page']))
    {
        $page = 'error.php'; 
    }
    
    elseif (!file_exists(ROOT."_pages/". $filename))
    {
        $page = 'error.php';       
    }
    else
    {
        $page = $_GET['page'] . '.php'; 
    }
    
return $page;   
}


// kijk in welke map er gekeken moet worden
(isset($_SESSION['section'])? ($sectie = $_SESSION['section']) : ($sectie = 'publiek'));

// kijk welk bestand uit die map geinclude moet worden
(isset($_GET['page'])? ($page = checkPage($sectie)) : ($page = 'home.php'));

include(ROOT .'_pages/'. $sectie .'/'. $page);



?>

[ Voor 21% gewijzigd door dossiewossie op 20-09-2005 15:55 ]

[Verwijderd]

Wat als $page nu ../../../../etc/passwd ofzoiets is (ik weet alleen van toepassing op *nix bakken) dan laat hij die file zien. Vandaar dat je op dit soort constructies moet testen. Bij php.net vind je hier ook dingen over.

[Verwijderd]

Kan je niet beter andersom werken, en juist de afzonderlijke pagina's gebruiken, en zaken die altijd moeten verschijnen / gebeuren includen vanaf die pagina's?

[dossiewossie]

Je hebt gelijk, dat zal ik moeten afvangen.

Ik kijk nog even verder op php.net

[dossiewossie]

Verwijderd schreef op dinsdag 20 september 2005 @ 14:09:
Kan je niet beter andersom werken, en juist de afzonderlijke pagina's gebruiken, en zaken die altijd moeten verschijnen / gebeuren includen vanaf die pagina's?

In de geinclude paginas worden ook weer bepaalde standaard delen geinclude.

Voor mijzelf vindt ik deze manier van werken iets overzichtelijker

[Erkens]

dossiewossie schreef op dinsdag 20 september 2005 @ 14:09:
Je hebt gelijk, dat zal ik moeten afvangen.

Ik kijk nog even verder op php.net

Wat je het beste kan doen is testen (met een regex ofzo) of die string alleen bijvoorbeeld a-z en 0-9 bevat en geen andere tekens als . en / etc.

Verwijderd schreef op dinsdag 20 september 2005 @ 14:09:
Kan je niet beter andersom werken, en juist de afzonderlijke pagina's gebruiken, en zaken die altijd moeten verschijnen / gebeuren includen vanaf die pagina's?

dan heb je niet een handig overzicht + extra werk als er een nieuwe pagina bijkomt.
Overigens is een groot voordeel van deze manier dat je met een eenvoudige mod_rewrite rule mooie URL's kan krijgen.

[T-MOB]

/etc/passwd krijg je niet te zien aangezien er .php wordt toegevoegd aan $page. Dat neemt niet weg dat op deze wijze in principe elke .php file op de server ingelezen kan worden. Met een beetje giswerk kan iederen nu dus bij je "beveiligde" pagina's.
Een beter methode is om alleen pagina's te includen die je ergens zelf definieert. Dat kan in een array of je database, net om hoeveel pagina's het gaat.

[dossiewossie]

Erkens schreef op dinsdag 20 september 2005 @ 14:11:
[...]

Wat je het beste kan doen is testen (met een regex ofzo) of die string alleen bijvoorbeeld a-z en 0-9 bevat en geen andere tekens als . en / etc.


[...]

dan heb je niet een handig overzicht + extra werk als er een nieuwe pagina bijkomt.
Overigens is een groot voordeel van deze manier dat je met een eenvoudige mod_rewrite rule mooie URL's kan krijgen.

Kijk daar zat ik net aan te denken, dankjewel

[dossiewossie]

T-MOB schreef op dinsdag 20 september 2005 @ 14:14:
/etc/passwd krijg je niet te zien aangezien er .php wordt toegevoegd aan $page. Dat neemt niet weg dat op deze wijze in principe elke .php file op de server ingelezen kan worden. Met een beetje giswerk kan iederen nu dus bij je "beveiligde" pagina's.
Een beter methode is om alleen pagina's te includen die je ergens zelf definieert. Dat kan in een array of je database, net om hoeveel pagina's het gaat.

Vergis je je niet? Ik heb het net geprobeerd, en dat schijnt echt niet te lukken hoor.

Kijk je niet over $sectie heen? Daar heeft de gebruiker geen invloed op.

Denk ik.......

[NMe]

T-MOB schreef op dinsdag 20 september 2005 @ 14:14:
/etc/passwd krijg je niet te zien aangezien er .php wordt toegevoegd aan $page.

Nou, ik weet het niet. Ik weet niet helemaal hoe include in elkaar zit, maar als het lijkt op de filesystem functies van Linux/UNIX zelf, dan kun je misschien wel een pipe-teken invoeren. .php erachter zetten zou in dat geval niets uithalen, en is nog steeds onveilig, maar of het echt zo werkt weet ik niet zeker.

Een beter methode is om alleen pagina's te includen die je ergens zelf definieert. Dat kan in een array of je database, net om hoeveel pagina's het gaat.

Inderdaad, sterker nog, AFAIK is dat de enige echt veilige manier: alle mogelijke opties in een array zetten, en kijken of de binnengekomen waarde in dat array zit. Zo niet: indexpagina weergeven. Zo ja: de opgevraagde pagina.

[Skaah]

PHP:

function checkPage($sectie)
{
    if (ctype_alnum($_GET['page']) {
        $filename = $sectie .'/'. $_GET['page'];
    }
    else {
        die('Doei!');
    }

    if (!file_exists(__ROOT__ ."_pages/". $filename))
    {
        $page = 'error.php';       
    }
    else
    {
        $page = $page .'.php';    
    }
    
return $page;    
}

[dossiewossie]

-NMe- schreef op dinsdag 20 september 2005 @ 14:20:
[...]

Nou, ik weet het niet. Ik weet niet helemaal hoe include in elkaar zit, maar als het lijkt op de filesystem functies van Linux/UNIX zelf, dan kun je misschien wel een pipe-teken invoeren. .php erachter zetten zou in dat geval niets uithalen, en is nog steeds onveilig, maar of het echt zo werkt weet ik niet zeker.

[...]

Inderdaad, sterker nog, AFAIK is dat de enige echt veilige manier: alle mogelijke opties in een array zetten, en kijken of de binnengekomen waarde in dat array zit. Zo niet: indexpagina weergeven. Zo ja: de opgevraagde pagina.

Als het echt niet kan, dan zal ik dat doen. Maar liever niet als het niet nodig is. Kan iemand nog een voorbeeldje geven waarom dit gevaarlijk zou zijn?

Ik kan natuurlijk ook de mappen in pages uitlezen, en voor elke map een array van alle paginas maken, Dat is ook nog wel flexibel.

Is dat een beter id?

[Verwijderd]

grote kans dat je dus iets kan doen als "../../../porno.jpg; een_php_bestand"

[dossiewossie]

Skaah schreef op dinsdag 20 september 2005 @ 14:23:

PHP:

function checkPage($sectie)
{
    if (ctype_alnum($_GET['page']) {
        $filename = $sectie .'/'. $_GET['page'];
    }
    else {
        die('Doei!');
    }

    if (!file_exists(__ROOT__ ."_pages/". $filename))
    {
        $page = 'error.php';       
    }
    else
    {
        $page = $page .'.php';    
    }
    
return $page;    
}

Ah, gewoon een php functie hiervoor

Prettig, regex vindt ik altijd beetje onoverzichtelijk.

Dit zal ik er in ieder geval ook ingooien, bedankt.

[Janoz]

ene kant
template:

code:

START
include iets
EIND

iets

code:

voorbereidend werk
helecontent

andere kant
iets

code:

voorbereidend werk
include START
hele content
include EIND

met uiteraard de bestanden START en EIND

dan heb je niet een handig overzicht

Waarom niet, het overzicht blijft exact hetzelfde. ipv 1 index.php is er nu een header.php en footer.php.

+ extra werk als er een nieuwe pagina bijkomt.

Het bestand dat je normaal include heeft inderdaad 2 regeltjes extra. Je template is echter compleet verschoont van include checks aangezien er nergens een pagina op basis van userinput geinclude moet worden. Daarnaast zijn fouten die in je de in jouw geval geinclude content optreden lastig af te handelen aangezien je header al verstuurt is door je index.php (tenzij je dat weer met ob_start ed afvangt)

Overigens is een groot voordeel van deze manier dat je met een eenvoudige mod_rewrite rule mooie URL's kan krijgen.

Bij de oplossing waarbij je je template include heb je al mooie URL's en hoef je die niet te simuleren met mod_rewrites.

[vinnux]

Of doe het geheel andersom maak aparte pagina's en include de index:

PHP:

<?//Dynamically find the config file and calculate the relative path for the config file
if (!defined('REL')) {  
  $rel                          = '';
  $REL_CONFIG_FILE  = 'config.inc.php';
  $REL_DEPTH            = 0;
  $REL_MAX_DEPTH        = 10;
  $REL_FOUND                = file_exists($rel.$REL_CONFIG_FILE);
    while (!$REL_FOUND && $REL_DEPTH < $REL_MAX_DEPTH) {
      $REL_DEPTH++;
        $rel.="../";
        $REL_FOUND = file_exists($rel.$REL_CONFIG_FILE);
    }
    if($REL_FOUND === true) {
        require_once($rel.$REL_CONFIG_FILE);
        unset($rel,$REL_DEPTH,$REL_MAX_DEPTH,$REL_FOUND);
    } else {
      //The config file could not be found. The page cannot bedisplayed
        die('FATAL : '.$REL_CONFIG_FILE.' not found');  
    }
} 
require_once(REL.'-inc-/pageGui.inc.php');              //Displaying functions ?>

[Brakkie]

Ik pak dit simpel gezegd altijd zo aan.

PHP:

$pages = array('index','page','fotoalbum');

if(in_array($_GET['page'], $pages))
{
     include($_GET['page']);
}
else
     echo "Pagina bestaat niet";

[ Voor 18% gewijzigd door Brakkie op 20-09-2005 14:42 ]

[Michali]

Die checkPage functie klopt sowieso niet helemaal volgens mij. Bij de laatste else staat nameljik dit:

PHP:

$page = $page . ".php";

terwijl $page helemaal geen waarde heeft gekregen eerder in de functie. Het is ook geen global, want ik zie er geen declaratie voor. Je kunt trouwens ook gemakkelijk met guard clauses werken. Dus:

PHP:

  return 'error.php';
}
else
{
  return $page . '.php';
}

En dan die laatste return weglaten. Scheelt je weer een overbodige regel.

[ Voor 5% gewijzigd door Michali op 20-09-2005 14:54 ]

[dossiewossie]

vgouw schreef op dinsdag 20 september 2005 @ 14:42:
Of doe het geheel andersom maak aparte pagina's en include de index:

PHP:

<?//Dynamically find the config file and calculate the relative path for the config file
if (!defined('REL')) {  
  $rel                          = '';
  $REL_CONFIG_FILE  = 'config.inc.php';
  $REL_DEPTH            = 0;
  $REL_MAX_DEPTH        = 10;
  $REL_FOUND                = file_exists($rel.$REL_CONFIG_FILE);
    while (!$REL_FOUND && $REL_DEPTH < $REL_MAX_DEPTH) {
      $REL_DEPTH++;
        $rel.="../";
        $REL_FOUND = file_exists($rel.$REL_CONFIG_FILE);
    }
    if($REL_FOUND === true) {
        require_once($rel.$REL_CONFIG_FILE);
        unset($rel,$REL_DEPTH,$REL_MAX_DEPTH,$REL_FOUND);
    } else {
      //The config file could not be found. The page cannot bedisplayed
        die('FATAL : '.$REL_CONFIG_FILE.' not found');  
    }
} 
require_once(REL.'-inc-/pageGui.inc.php');              //Displaying functions ?>

hmmm, dat zou misschien makkelijker werken jah, maar ik zie niet snel genoeg wat het doet. Ik ben alleszins geen php guru, ik hou het liever bij code die ik in een oogopslag snap.

[dossiewossie]

Michali schreef op dinsdag 20 september 2005 @ 14:54:
Die checkPage functie klopt sowieso niet helemaal volgens mij. Bij de laatste else staat nameljik dit:

PHP:

$page = $page . ".php";

terwijl $page helemaal geen waarde heeft gekregen eerder in de functie. Het is ook geen global, want ik zie er geen declaratie voor. Je kunt trouwens ook gemakkelijk met guard clauses werken. Dus:

PHP:

  return 'error.php';
}
else
{
  return $page . '.php';
}

En dan die laatste return weglaten. Scheelt je weer een overbodige regel.

Ja, was ff een foutje denk ik. Heb ik altijd als ik chaotisch loop te klooten

[T-MOB]

-NMe- schreef op dinsdag 20 september 2005 @ 14:20:
[...]
Nou, ik weet het niet. Ik weet niet helemaal hoe include in elkaar zit, maar als het lijkt op de filesystem functies van Linux/UNIX zelf, dan kun je misschien wel een pipe-teken invoeren. .php erachter zetten zou in dat geval niets uithalen, en is nog steeds onveilig, maar of het echt zo werkt weet ik niet zeker.

code:

Warning: main(index.php | .php): failed to open stream: No such file or directory in /var/www/test.php on line 3

dossiewossie schreef op dinsdag 20 september 2005 @ 14:18:
Vergis je je niet? Ik heb het net geprobeerd, en dat schijnt echt niet te lukken hoor.

Kijk je niet over $sectie heen? Daar heeft de gebruiker geen invloed op.

Toen ik dat schreef stond er nog geen ctype_alnum() in je code. Dan had je dus als pagina "../een_gegokte_section/page" kunnen opgeven

[vinnux]

dossiewossie schreef op dinsdag 20 september 2005 @ 15:12:
[...]hmmm, dat zou misschien makkelijker werken jah, maar ik zie niet snel genoeg wat het doet. Ik ben alleszins geen php guru, ik hou het liever bij code die ik in een oogopslag snap.

Daar hebben ze commentaar voor uitgevonden.
Het zoekt naar "config.inc.php" file in de onderliggende directory en dat maximaal 10 niveaus diep.
Daarna vult hij de variable REL met het reletive path naar die config.inc.php

[simon]

vgouw schreef op dinsdag 20 september 2005 @ 14:42:
Of doe het geheel andersom maak aparte pagina's en include de index:

PHP:

<?//Dynamically find the config file and calculate the relative path for the config file
if (!defined('REL')) {  
  $rel                          = '';
  $REL_CONFIG_FILE  = 'config.inc.php';
  $REL_DEPTH            = 0;
  $REL_MAX_DEPTH        = 10;
  $REL_FOUND                = file_exists($rel.$REL_CONFIG_FILE);
    while (!$REL_FOUND && $REL_DEPTH < $REL_MAX_DEPTH) {
      $REL_DEPTH++;
        $rel.="../";
        $REL_FOUND = file_exists($rel.$REL_CONFIG_FILE);
    }
    if($REL_FOUND === true) {
        require_once($rel.$REL_CONFIG_FILE);
        unset($rel,$REL_DEPTH,$REL_MAX_DEPTH,$REL_FOUND);
    } else {
      //The config file could not be found. The page cannot bedisplayed
        die('FATAL : '.$REL_CONFIG_FILE.' not found');  
    }
} 
require_once(REL.'-inc-/pageGui.inc.php');              //Displaying functions ?>

jeetje, dat is omslachtig, zoveel onnodige code.... echt sneller/overzichtelijker wordt je code er ook niet van

[vinnux]

Maar wel heeeeeeeeel erg makkelijk, ik kan zo hele directories verplaatsen hernoemen etc zonder één klein probleem. En het moest veilig toch .. nou dit was de veiligste oplossing. Gebruik het al jaren op kleine en groote sites en nog nooit problemen mee gehad.
En op deze manier kan ik ook meerdere sites op één site zetten.

Tevens kan ik op deze manier ook meerdere config files gebruiken die bepaalde waardes overschrijft in config file in de root bijvoorbeeld.

[ Voor 18% gewijzigd door vinnux op 20-09-2005 15:26 ]

[dossiewossie]

T-MOB schreef op dinsdag 20 september 2005 @ 15:17:
[...]

code:

Warning: main(index.php | .php): failed to open stream: No such file or directory in /var/www/test.php on line 3

[...]


Toen ik dat schreef stond er nog geen ctype_alnum() in je code. Dan had je dus als pagina "../een_gegokte_section/page" kunnen opgeven

Jeps, dat snap ik, maar voordat ik het erbij had gezet heb ik het getest, en toen werkte het ook niet. Misschien omdat ik op een windows machine test, anders weet ik het ook niet

[Erkens]

-NMe- schreef op dinsdag 20 september 2005 @ 14:20:
Inderdaad, sterker nog, AFAIK is dat de enige echt veilige manier: alle mogelijke opties in een array zetten, en kijken of de binnengekomen waarde in dat array zit. Zo niet: indexpagina weergeven. Zo ja: de opgevraagde pagina.

onzin, als je checked op chars in die string dat ze alleen a-z en 0-9 etc mogen zijn en dan kijken of die file exists ben je net zo veiliger, met als grote pluspunt dat je gewoon een file kan toevoegen zonder iets te editen en het werkt

[Grijze Vos]

Overigens, waarom gebruik je die __ __ in je constante? Je weet dat die doorgaans gebruikt worden voor "magic constants"?

[dossiewossie]

Grijze Vos schreef op dinsdag 20 september 2005 @ 15:44:
Overigens, waarom gebruik je die __ __ in je constante? Je weet dat die doorgaans gebruikt worden voor "magic constants"?

En verandert

Ik als beginnende phper weet een beetje maar bij lange na niet alles, even gechecked op php.net, en inderdaad, dit wordt voor magic constants gebruikt. Nu is het weer wat netter.

thnx

[PrisonerOfPain]

-NMe- schreef op dinsdag 20 september 2005 @ 14:20:
[...]

Nou, ik weet het niet. Ik weet niet helemaal hoe include in elkaar zit, maar als het lijkt op de filesystem functies van Linux/UNIX zelf, dan kun je misschien wel een pipe-teken invoeren. .php erachter zetten zou in dat geval niets uithalen, en is nog steeds onveilig, maar of het echt zo werkt weet ik niet zeker.

Null character (%00 urlencoded) toevoegen aan het eind van de parameter heeft ook zijn nut. Alleen zal de check die uitgevoerd word door ctype_alphanum struikelen over de / in je string.

[dossiewossie]

PrisonerOfPain schreef op dinsdag 20 september 2005 @ 16:44:
[...]

Null character (%00 urlencoded) toevoegen aan het eind van de parameter heeft ook zijn nut. Alleen zal de check die uitgevoerd word door ctype_alphanum struikelen over de / in je string.

Bedoel je mijn string? Alleen de $_GET['page'] wordt gechecked hoor, niet het hele pad

[PrisonerOfPain]

dossiewossie schreef op dinsdag 20 september 2005 @ 16:49:
[...]


Bedoel je mijn string? Alleen de $_GET['page'] wordt gechecked hoor, niet het hele pad

pagina.php?page=/etc/passwd%00 zal een error.php opleveren enkel en alleen vanwegen de / in het pad. Als die er niet zou zijn, en ik bijvoorbeeld geinteresseerd zou zijn in miep (zonder .php, dan zou ctype_alphanum namelijk over de . struikelen) zou ik pagina aanroepen met pagina.php?page=miep%00 in dat geval word miep, en niet miep.php geinclude vanwege de manier waarop PHP met strings werkt. (Namelijk, een \0 of null byte of %00 in het geval van urlencoding allemaal hetzelfde , geeft het einde van een string aan.) Zie bijvoorbeeld ook http://bugs.php.net/bug.php?id=30489.

Toch maar even getest, blijkt dat ctype_alphanum ook over \0 struikelt, hier tenminste.

[ Voor 8% gewijzigd door PrisonerOfPain op 20-09-2005 17:05 ]

[Skaah]

dossiewossie schreef op dinsdag 20 september 2005 @ 14:27:
[...]
Ah, gewoon een php functie hiervoor
Prettig, regex vindt ik altijd beetje onoverzichtelijk.
Dit zal ik er in ieder geval ook ingooien, bedankt.

Onoverzichtelijk?

PHP:

preg_match("^[A-z0-9]+$",$string);

Valt wel mee toch? Alleen is ctype_alnum() (1000x ofzo) sneller.

[PrisonerOfPain]

Skaah schreef op woensdag 21 september 2005 @ 10:44:
Valt wel mee toch?

Inderdaad, maargoed, de regel dat wat men niet beheerst, is onduidelijk, is vaak de reden. Overigens kunnen named sets al een hoop verduidelijken aan je regexp, en kun je ook daarin commentaar gebruiken en nette indenting gebruiken. Dus dat zal het probleem dan ook weer niet hoeven zijn. Wat ik onoverzichtelijk vind, is het gebruik van vijfentwintig PHP functies om hetzelfde te doen als een regular expression.

[Skaah]

Je kan nu trouwens nog steeds lege pagina's includen of een fout oproepen door geen GET variabele aan de URL mee te geven.

[Creepy]

Even wat replies afgesplitst naar [rml][ alg] Commentaar in code nodig of overbodig?[/rml]