[PHP] codeinjection via $_POST beveiligen?

[moozzuzz]

PHP:

if (!empty($_GET)) {
    while (list($name, $value) = each($_GET)) 
    {
        $$name = $value;
    }
}

Hey hey, ik gebruik al jaaaaren deze constructie voor $_POST en $_GET, maar vroeg me onlangs af of dit eigenlijk een veilige manier van werken is. Stel dat de name van de vorm

code:

'blah; <insert evil code here> $blah'

Ik kijk misschien op de verkeerde plek op php.net, maar vind hier niet meteen iets terug? Als ik een testje doe:

PHP:

// met HTML: <input name="blah; include('.htaccess'); insert evil = code here; $blah" value="other evil">
echo '<b>$'.$name.'</b> = '.$value;

... krijg ik:

$blah;_include('_htaccess');_insert_evil_=_code_here;_$blah = other evil

Hoe kan deze constructie beter beveiligd worden? [of kan iemand me geruststellen :^) Wellicht stel ik hier de domste vraag van de week]

[kmf]

als het om een numerieke waarde gaat, altijd intval gebruiken.

als het om andere waardes gaat, meerdere oplossingen mogelijk. Ligt maar aan waar je de input voor nodig heb. Escapen als je niet kan voorspellen wat voor waardes men gaan invoeren (forumposts, comments) of een switch gebruiken als je wel weet wat de mogelijke input zijn (alleen iets doen als de waarde in de switch voorkomt)

[Verwijderd]

Als je een $_GET of $_POST direct weergeeft op een pagina, kan je dat het beste doen op de volgende manier:

PHP:

<?php
echo htmlentities($_GET['iets']);
// Syntax: string htmlentities ( string string [, int quote_style [, string charset]] )
?>

Domste vraag van de week? Vast niet. Het is natuurlijk altijd goed als je het beveiligd, maar het is denk ik niet voor een groot project ofzo? Want dit is alleen gevoelig voor XSS (Cross-Site-Scripting).

Mocht je met SQL Query's rechtstreeks $_GET of $_POST gebruiken, dan heeft dat een veel groter risico (SQL Injection) en dit kan je het best voorkomen door mysql_real_escape_string() gebruiken, zoals hieronder:

PHP:

<?php
$fout = 'SELECT * FROM tabelnaam WHERE naam = "'.$_GET['naam'].'"';
$goed = 'SELECT * FROM tabelnaam WHERE naam = "'.mysql_real_escape_string($_GET['naam']).'"';
// string mysql_real_escape_string ( string unescaped_string [, resource link_identifier] )
?>

Succes!

[Radiant]

Ik vraag me eigenlijk uberhaupt af waarom je dit zo doet. Als je dit echt zo graag wilt, waarom gebruik je dan niet register_globals? Of haal gewoon direct je data uit je $_POST en $_GET arrays?

[Genoil]

De constructie die je toepast kan ook op deze manier tot onveilige situaties leiden:

PHP:

$isAdmin = $_SESSION['isAdmin'];

if (!empty($_GET)) {
    while (list($name, $value) = each($_GET)) 
    {
        $$name = $value;
    }
}

Als ik nu www.website.com/index.php?isAdmin=1 doe, ben ik admin zonder dat het de bedoeling is. Of www.website.com/index.php?_SESSION['isAdmin']=1. Sowieso kan zonder enige toevoeging je pagina 'crashen' door deze url: www.website.com/index.php?_GET=1. Da's niet echt fraai...

[ Voor 9% gewijzigd door Genoil op 12-11-2006 11:31 ]

[Michali]

a. Hier heb je een standaard functie voor: http://nl2.php.net/manual...ort-request-variables.php

b. Niet gebruiken, of je moet echt niet zonder kunnen (en dat is alleen in het geval dat je een app. hebt die met register globals aan is ontwikkeld).

[PeterSelie]

misschien dat dit een leuke is, het behandeld alle gevaarlijke punten van brak PHP scripting en geeft ook gelijk een oplossing hoe dit tegen te gaan:

http://www.phpfreakz.nl/artikelen.php?aid=106

[Raynman]

Genoil schreef op zondag 12 november 2006 @ 11:26:
De constructie die je toepast kan ook op deze manier tot onveilige situaties leiden:

PHP:

$isAdmin = $_SESSION['isAdmin'];

if (!empty($_GET)) {
    while (list($name, $value) = each($_GET)) 
    {
        $$name = $value;
    }
}

Als ik nu www.website.com/index.php?isAdmin=1 doe, ben ik admin zonder dat het de bedoeling is. Of www.website.com/index.php?_SESSION['isAdmin']=1. Sowieso kan zonder enige toevoeging je pagina 'crashen' door deze url: www.website.com/index.php?_GET=1. Da's niet echt fraai...

Maar het is ook al onzin om nog een variabele te maken als je $_SESSION['isAdmin'] al hebt.

Zelfde geldt inderdaad voor GET en POST waardes. Deze constructie is meer een quick-and-dirty oplossing om oude scripts op nieuwere servers te laten werken met register_globals=off.

Hoe kan deze constructie beter beveiligd worden? of kan iemand me geruststellen :^) Wellicht stel ik hier de domste vraag van de week

Zolang je het gewoon als tekst beschouwt/gebruikt is het toch helemaal niet erg dat er "include '.htaccess';" in staat. Pas als je het als PHP-code gaat gebruiken (door eval() of opslaan in bestand en dat includen) moet je goed op beveiliging gaan letten, maar dat is logisch.
Als je geen htmlspecialchars() (of htmlentities) gebruikt, kunnen er misschien wel trucjes uitgehaald worden m.b.v. JavaScript.

[Confusion]

Raynman schreef op zondag 12 november 2006 @ 12:31:
Maar het is ook al onzin om nog een variabele te maken als je $_SESSION['isAdmin'] al hebt.

Totdat door voortschrijdend inzicht isAdmin' wordt hernoemd naar 'hasAdminRights' en jij op zestien plaatsen in je code de string 'isAdmin' moet gaan veranderen. Dan is het makkelijk om dat bovenaan je script maar 1 keer te hoeven doen, terwijl de variabelenaam 'isAdmin' voor dat deel van het script misschien wel afdoende is.

[crisp]

Confusion schreef op zondag 12 november 2006 @ 12:44:
[...]

Totdat door voortschrijdend inzicht isAdmin' wordt hernoemd naar 'hasAdminRights' en jij op zestien plaatsen in je code de string 'isAdmin' moet gaan veranderen. Dan is het makkelijk om dat bovenaan je script maar 1 keer te hoeven doen, terwijl de variabelenaam 'isAdmin' voor dat deel van het script misschien wel afdoende is.

Dan nog is een global var gewoon ugly en kan je beter een method of functie gebruiken:

PHP:

function hasAdminRights()
{
    return !empty($_SESSION['hasAdminRights']);
}

verder is de manier om je te beveiligen tegen vormen van code-insertion gewoon ervoor zorgen dat je naar alle vormen van output user-generated content altijd op de juiste manier escaped/encode en bij invoer ook altijd sanity-checks uitvoert.

[moozzuzz]

OKI, duidelijk bad practice... Zal voor de toekomst (het blijven hobby-sitekepjes) hier niet meer aan zondigen ;^)

Mijn belangrijkste zorg was of echter of er door de constructie $$ geen rare dingen konden gebeuren. Dat blijkt (bij mij?) toch niet zo te zijn.

PHP:

<html><head></head><body><?php
if (!empty($_POST)) {
    while (list($name, $value) = each($_POST)) 
    {
        $$name = $value;
        echo '<br>$$name = $value;'; // door PHP verwerkt geeft dit immers:
        echo '<br>$<b>'.$name.'</b> = '.$value; 
        // zou moeten printen 'blah : other evil', maar print niets (fieuw)
        echo '<br>blah : '.$blah; 
    }
}
?>

<form action="evilcode-test.php" method=post>
<input name="blah=1;printf($blah);$blah" value="'other evil';">
<input type=submit>
</form></body></html>

Alvast bedankt voor de input!

[Verwijderd]

Ik heb nog nooit $$ gebruikt, en dat zal ik ook nooit doen. Zo ongeveer de enige reden die ik kan bedenken om dat te willen gebruiken is het niet kunnen werken met arrays. Het slaat nergens op. Als je dan inderdaad er bang voor bent dat je de naam van een request variabele een keer moet veranderen, gebruik dan constants als key.

PHP:

doe_iets_met ( $_POST [ EEN_OF_ANDERE_CONSTANTE ] );

Beter nog:

PHP:

doe_iets_met ( $request->get ( EEN_OF_ANDERE_CONSTANTE ) );

Zo hoef je niet overal te kloten met de superglobals, en kun je eventueel ook een keer een ander object als $request argument aan een functie passen.