Er dette include-scriptet sikkert?

[ThomasQ]

Mekket et includescript nå

 

Lurer på om det finnes noen svakheter i det? Evt, noen ting som kunne vært bedre.

 

<?php

$mappe = "sider/";
$default = "index";
$fil = ".php";
$side = trim($_GET['side']);

if(!$side) {
	include_once $mappe.$default.$fil;
}

$fa_side = $mappe.$side.$fil;

if($side) {

if(strpos($side, '/') || strpos($side, '\\') || strpos($side, '..')) {
	die("<h1>Ulovlige tegn i adresse!</h1>");
}	

if(!file_exists($fa_side)) {
	die("<h1>Beklager, siden du leter etter finnes ikke!<br>Vennligst gå tilbake til forsiden, å prøv igjen!</h1>");
}
	include_once $mappe.$side.$fil;
}

?>

Endret 1. desember 2008 av ThomasQ

[Ernie]

Ikke sikkert i det heltatt. Er $_GET['side'] null terminert dropper både file_exists og include å ta med .php, og da kan man jo i praksis inkludere hva man ønsker. I tillegg går det fult an å hente inn filer i mappestrukturen både over og under sider ved å bruke ..

 

Det du altså må gjøre er å sjekke at den ikke inneholder null-byte og ..

[ThomasQ]

Ikke sikkert i det heltatt. Er $_GET['side'] null terminert dropper både file_exists og include å ta med .php, og da kan man jo i praksis inkludere hva man ønsker. I tillegg går det fult an å hente inn filer i mappestrukturen både over og under sider ved å bruke ..

 

Det du altså må gjøre er å sjekke at den ikke inneholder null-byte og ..

 

Gi et eksempel på hvordan jeg kan inkludere hva som helst?

[Ernie]

../mappePåSammeNivåSomSider/sensitivFil\ 0

Dette vil medføre at du inkluderer "sider/../mappePåSammeNivåSomSider/sensitivFil\ 0.php" som i øynene på både file_exists og include blir det samme som "mappePåSammeNivåSomSider/sensitivFil". I eksemplet gikk jeg bare et hakk opp i mappestrukturen, men det er ingenting i veien for å bruke

../../../../../../mappe/fil\ 0

, havne 5 mapper opp i strukturen og hente inn fil i mappen mappe. Bytt ut mappe med etc, fil med passwd og få antall ../ riktig så kan man hente inn /etc/passwd, og det et aldri så lite sikkerhetsproblem.

 

Red.: Bruk \ 0 for nullbyte.

Endret 30. november 2008 av Ernie

[ThomasQ]

Hvordan skal jeg forhindre det?

[Ernie]

Hvordan skal jeg forhindre det?

 

Det du altså må gjøre er å sjekke at den ikke inneholder null-byte og ..

[ThomasQ]

Hvordan skal jeg forhindre det?

 

Det du altså må gjøre er å sjekke at den ikke inneholder null-byte og ..

 

Forventer du at alle kan det?

 

empty() da?

[Ernie]

Vel, det er jo ganske enkel bruk av strpos da. Et nullbyte-tegn får du via "\" eller chr(0), og .. sier seg selv.

[nree]

if(strpos($side, '/') || strpos($side, '\') || strpos($side, '..')) {
die("Ulovlige tegn i adresse!");
}

[ThomasQ]

if(strpos($side, '/') || strpos($side, '\') || strpos($side, '..')) {
 die("Ulovlige tegn i adresse!");
 }

 

Takk

 

 

strpos($side, '\')

 

Fungerer ikke:

Parse error: syntax error, unexpected '.' in C:\wamp\www\design 15\index.php on line 16

Endret 1. desember 2008 av ThomasQ

[Ernie]

det skal være "\ 0" minus mellomrommet. Forumet takler ikke at man skriver \ 0 :-/

[Martin A.]

Vil ikke en preg_match på alle tegn unntatt a-zA-Z0-9 løse dette?

[Ernie]

Joa, men da kan man jo likegodt bruke ctype_alnum.

[ThomasQ]

det skal være "\ 0" minus mellomrommet. Forumet takler ikke at man skriver \ 0 :-/

 

\ 0 fungerer jo like dårlig..

Endret 1. desember 2008 av ThomasQ

[shaker]

feil tråd

Endret 1. desember 2008 av shaker

[Jonas]

Du har en syntaksfeil! Nå er det back to the basics, her. Og dersom du forventer at vi skal vise deg hvor denne feilen ligger, så er du nødt til å vise oss nøyaktig hva som står på linje 14-18.

 

http://www.google.no/search?hl=no&q=ph...3%B8k&meta=

 

Edit: «Fungerer dårlig» sier ingen her noe som helst, så vær mer spesifikk.

Endret 1. desember 2008 av Jonas

[ThomasQ]

Du har en syntaksfeil! Nå er det back to the basics, her. Og dersom du forventer at vi skal vise deg hvor denne feilen ligger, så er du nødt til å vise oss nøyaktig hva som står på linje 14-18.

 

http://www.google.no/search?hl=no&q=ph...3%B8k&meta=

 

Edit: «Fungerer dårlig» sier ingen her noe som helst, så vær mer spesifikk.

 

if(strpos($side, '/') || strpos($side, '\') || strpos($side, '..')) {

[Ernie]

'\' må være "\ 0" (uten mellomrom), altså med dobbeltfnutter rundt (de her: " " )

[Epower]

'\' må være "\ 0" (uten mellomrom), altså med dobbeltfnutter rundt (de her: " " )

Er det ikke bedre å bruke chr(0) da?

[Ernie]

Joda, det er jo forsåvidt likegreit.