[PHP] Unsafe use of variable in call include()/require()

Pagina: 1
Acties:

Onderwerpen


Acties:
  • 0 Henk 'm!

  • vorlox
  • Registratie: Juni 2001
  • Laatst online: 02-02-2022

vorlox

I cna ytpe 300 wrods pre miute

Topicstarter
He,

Ik heb zend studio gekocht (stom stom stom) dus ik ben er maar mee gaan werken.
Via de "Analyze code" button heb ik al een aantal dingtjes weggepoetst echter ik blij de volgende melding houden

Unsafe use of variable in call include()/require() (line 796)

Nu geeft de zend documentatie aan dat je voor een include/require het volgende moet doen.

PHP:
1
2
define('SCRIPT_PATH', '/htdocs');
include(SCRIPT_PATH.'/foo.inc')


Das natuurlijk een leuk voorbeeld maar ik heb juist een variabele include nodig.

Dus zoiets als
PHP:
1
2
3
4
5
6
7
$invoke = !empty($_GET['invoke']) ? $_GET['invoke'] : NULL;
if(CheckIfGebruikerMagHierbij($invoke))
    $file = !is_null($invoke) ? 'invokes/'.$invoke.'.php' : NULL;
    if( !is_null($file) && file_exists($file) {
        require($file);
    }
}


Nu is bovenstaand voorbeeld nog niet helemaal secur, maar het gaat even om het voorbeeld.
_autoload() werkt voor mij niet in dit geval omdat het geen class bevat maar gewoon php code.

Verder kan ik eigenlijk alleen maar foute voorbeelden vinden.
Wat is de beste manier om dit aan te pakken...of gewoon zend melding ignoren ;)

Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 02:21

Janoz

Moderator Devschuur®

!litemod

Nee, een switch gebruiken zodat je de mogelijke waarden kunt beperken tot de waarden die je ook daadwerkelijk wilt gebruiken.

Een andere mogelijkheid is om het includen andersom te doen. Dus niet de juiste 'invoke' zoeken middels een get parameter, maar je 'index' includen in je 'invoke'.

[ Voor 40% gewijzigd door Janoz op 16-12-2008 11:41 ]

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

  • Voutloos
  • Registratie: Januari 2002
  • Niet online
Nee, dit moet je (&^$#^$^@$ niet negeren. Dmv relatieve paden kan nu elk willekeurig bestand geinclude worden.

{signature}


Acties:
  • 0 Henk 'm!

  • vorlox
  • Registratie: Juni 2001
  • Laatst online: 02-02-2022

vorlox

I cna ytpe 300 wrods pre miute

Topicstarter
Ik snap wat je bedoelt. Echter ik heb alle mogelijke invokes / dus relative file paths in de database zitten met daaraan rechten gekoppeld.
Ik check dus of de gebruiker rechten heeft voor de opgevraagde $invoke.
Hiermee sluit ik dus alle andere waarden uit....een extended switch zeg maar naar de database.
Een andere mogelijkheid is om het includen andersom te doen. Dus niet de juiste 'invoke' zoeken middels een get parameter, maar je 'index' includen in je 'invoke'.
Grappig je draait het om...is echter even te veel werk om hier toe te passen.

[ Voor 30% gewijzigd door vorlox op 16-12-2008 11:44 ]


Acties:
  • 0 Henk 'm!

  • Voutloos
  • Registratie: Januari 2002
  • Niet online
Tja, die code stond niet in de ts. ;) Maar dan nog kan die controle functie een nieuwe var teruggeven zodat je niet meer met de originele GET param werkt. Sowieso is het fijn om de rauwe user input niet overal in je script op te zien duiken. Gewoon aan het begin sanitizen/vervangen door vaste waardes.

{signature}


Acties:
  • 0 Henk 'm!

  • vorlox
  • Registratie: Juni 2001
  • Laatst online: 02-02-2022

vorlox

I cna ytpe 300 wrods pre miute

Topicstarter
KLopt heb net even toegevoegd. verder werk ik idd niet met GET, REQ en POST vars maar dit is dan ook pseudo code...om het voorbeeld aan te halen.
Dus heb nu ff (CheckIfGebruikerMagHierbij) erbij gezet....het blijft pseudo overigens...hehe


in het echt heet dat ding getInvoke($invoke) en returned een invoke object{
=>name
=>path
=>roles = array()
}

en die gaat weer door validateInvoke(Invoke $invoke) welke true / false returned

Dus die path uit het invoke object die include ik dus...echter daarop krijg ik dus de melding waarmee ik de topic inging.

include($invoke->path);

[ Voor 42% gewijzigd door vorlox op 16-12-2008 12:58 ]


Acties:
  • 0 Henk 'm!

  • MLM
  • Registratie: Juli 2004
  • Laatst online: 12-03-2023

MLM

aka Zolo

mja, als je 100% zeker bent dat je altijd veilige informatie in je path variabele hebt staan, kan je de waarschuwing negeren.
het is een waarschuwing, en de bedoeling ervan is dat je checkt of je zeker weet dat je code veilig is. nu je dat gedaan hebt, is het doel van die tool bereikt lijkt me ;)

-niks-


Acties:
  • 0 Henk 'm!

  • vorlox
  • Registratie: Juni 2001
  • Laatst online: 02-02-2022

vorlox

I cna ytpe 300 wrods pre miute

Topicstarter
Nou ja het zette me aan het denken, en idd hoop je dan maar dat je het 100% afgevangen hebt.
Verder vind ik die opmerking van Janoz wel mooi eigenlijk...als je het omdraait heb je dit probleem niet?

Hmm stof tot nadenken.

Het idee van het ontwerp is alsvolgt.
Je hebt kernel.php (DB , Environment checks e.d)
Je hebt een html.php, xml.php, pdf.php enz. Als je deze bestanden aanroept maken ze allemaal 1 instantie van de kernel.
Verder kun je dan met xml.php?invoke=doeiets min of meer een class laden in de kernel.
De kernel kijkt naar via welk bestand de invoke is aangeroepen dus zegmaar xml.php en laadt parseXml() uit de doeits class. Echter als ik zou aanroepen pdf.php?invoke=doeiets dan roept de kernel parsePdf() aan in de zelfde class. De invoke class moet voldoen aan een interface zodat de kernel voordat er output gegeven wordt dingen kan aanroepen zoals b.v init() initBeforePost() en als laatste dus parseXml(), parseHtml() enz.

Je zou natuurlijk ook alles van de kernel kunnen overerven...omdraaien dus...echter houd mijn xml.php enz dan geen stand.. of ik moet daar al iets doen met een variabele include...en dan heb ik weer hetzelfde probleem.

[ Voor 14% gewijzigd door vorlox op 16-12-2008 13:15 ]

Pagina: 1