CONDAMNER LA FAILLE INCLUDE()
cs_PaDa
Messages postés
1804
Date d'inscription
mardi 15 juillet 2003
Statut
Membre
Dernière intervention
22 septembre 2009
-
31 mai 2006 à 11:38
webdeb Messages postés 488 Date d'inscription samedi 5 avril 2003 Statut Membre Dernière intervention 31 mars 2009 - 26 août 2006 à 01:20
webdeb Messages postés 488 Date d'inscription samedi 5 avril 2003 Statut Membre Dernière intervention 31 mars 2009 - 26 août 2006 à 01:20
Cette discussion concerne un article du site. Pour la consulter dans son contexte d'origine, cliquez sur le lien ci-dessous.
https://codes-sources.commentcamarche.net/source/37874-condamner-la-faille-include
https://codes-sources.commentcamarche.net/source/37874-condamner-la-faille-include
26 août 2006 à 01:20
++
11 juil. 2006 à 18:44
en débutant que je suis, j'ai eu la chance de tomber sur une bonne d'emblée, le switch, celle de williamprotti, et sans savoir qu'elle était si bonne...
6 juin 2006 à 14:09
@ tchaOo°
6 juin 2006 à 11:15
On peut pas le faire aussi en version POO ? ;)
5 juin 2006 à 22:54
il y a dans les commentaires du probleme une solution pour les page par "URL"
5 juin 2006 à 10:34
dsl
5 juin 2006 à 09:26
donc :
<? include($mon_include); ?>
dsl ^^
5 juin 2006 à 09:24
<? //voila le code qui traite la variable $_GET[page]
$page=$_GET[page];
if ($page=="1") $mon_include="un_sous_rep/page1.php";
elseif ($page=="2") $mon_include="un_sous_rep/page2.php";
elseif ($page=="3") $mon_include="un_autre_sous_rep/un_sous_sous_rep/page456.php";
// etc...
else $mon_include="page_par_defaut.php"; // optionnel suivant l'utilisation de la page a inclure
?>
aprés suffi d'appellé la page par mapage.php?page=1 pour etre sur d'inclure la bonne page,
de cette facon impossible pour quelqu'un d'inclure une autre page que celle prevu par le developpeur.
2 juin 2006 à 15:04
Encore merci à tous !!!!!
@ Kankrelune
Mon avatar, c'est Tony Soprano, de la série Les Soprano ;))
Excellent !!!! ;)
1 juin 2006 à 20:43
php accède non seulement à http, mais aussi à ftp, et bien d'autres méthodes... (genre ogg ect...)
donc, faut pas utiliser ça, car si jamais une méthode est ajoutée, bah ton script ne marche plus...
un endroit ou je détailes un certain nombre de failles : http://coucou747.hopto.org/article_21.html
bon, selon moi, rien ne vaut le uin_array pour faire ça...
1 juin 2006 à 17:58
Dans d'autres parties, je peux avoir a les utiliser, donc il n'est pas systématiquement avantageux de gagner du temps ici si c'est pour en perdre plus bas. Par contre, les constantes ont comme avantage de me signaler si j'essaie de les déclarer plusieurs fois, alors qu'avec les variables, on écrase l'ancienne valeur sans aucun message d'avertissement, bien sur (et heureusement...)
Mais dans un contexte plus général c'est vrai que ce sont des stats très interessantes.
Ca me fait penser que j'ai quelques pages de source a revoir du coup...
Merci pour les précisions
1 juin 2006 à 17:41
while($i<100000)
{
if(isset($tableau['TEST']))
{
$var=$tableau['TEST'];
}
$i++;
}
@ tchaOo°
1 juin 2006 à 17:40
Kankrelune et les autres: on arrête de fantasmer svp!! Je ne suis pas un admin facile... ;-)
1 juin 2006 à 17:08
while($i<100000)
{
if(defined('TEST'))
{
$var=constant('TEST');
}
$i++;
}
while($i<100000)
{
if(in_array('TEST',$tableau))
{
$var=$tableau['TEST'];
}
$i++;
}
C'est le tableau qui est le plus rapide dès que tu récupères la valeur effectivement, dans le cas contraire la constante reste plus rapide ;)
1 juin 2006 à 16:49
@ tchaOo°
1 juin 2006 à 16:43
1 juin 2006 à 16:23
1 juin 2006 à 14:47
Mais c'est vrai que je pense pas que constant($page) soit plus rapide que $tab[$page].
J'ai jamais vraiment su à quel moment il était meilleur d'utiliser des constantes ou des variables...
1 juin 2006 à 14:35
Fais attention Malalam... tu ne connais pas nos désir les plus secrets... lOo°oOl... aïe... non... aïe... pô taper... pô la tête... .. . :oD
@ tchaOo°
1 juin 2006 à 14:33
$site_pages array('index'> 'index.php',
'chapitre1' => 'chapitres/chapitre1.php',
...
);
...
if (isset($site_pages[$page]))
include($site_pages[$page]);
else
include('fichiererreur.php');
@ TheSin
"Le .htaccess en plus, faudrait que j'y pense, mais comme je fonctionne sur des serveurs sur lesquels on ne peut inclure par défaut des fichiers qui ne nous appartiennent pas, je pense que j'ai pas de soucis de ce côté là"
Le .htaccess c'est juste pour que l'on ne puisse pas accéder au pages incluses directement... d'une part ça évite un affichage de ces pages sans header et footer... d'autre part ça évite l'affichage d'eereur (vu qu'il manque des fichiers inclu) qui peuvent renseigner un éventuel hackeur (notament en dévoilant le root du server)
@ xactise... is_file et file_exists c'est du pareil au même is_file étant légerement plus rapide... par contre c'est interessant ton histoire d'url rewriting faudra que je teste... .. .
J_G... ça a l'air pas mal comme technique... un peu plus lourd à mettre en place mais pas mal... .. . ;o)
@ killermaster...
"pour améliorer la faille, je vais pas avoir le temps désolé"
C'est pas la faille qu'il faut améliorer mais là façon de l'éviter... lol... et puis c'est pas pour le temps que ça prend... rooooooooh... .. .
Sympa ton avatar... tu fais vieu pour 17 ans... lol... bon ok je sors... .. . ;o)
@ tchaOo°
1 juin 2006 à 14:07
1 juin 2006 à 13:51
header("Location: /page_à_la_racine_du_site.php");
Ce n'est pas une erreur... Et je ne connais pas de navigateur ne comprennant pas ce message. Quel est le problème ?
Quique pour tout te dire, généralement, je définie au minimum ces 2 variables : SERVER_ROOT et HTTP_ROOT
Concernant le portage, personnelement, je ne ferais pas une classe de... cette unique fonction !
Ne vaut-il pas mieux mettre ces petits bouts de code dans un 'acces.inc.php' ?
Enfin, si t'es motivé pour la transformation... Pas de problème, je te file les droits d'exploitation ;)
1 juin 2006 à 13:30
je voulais dire "un portage" ;-)
1 juin 2006 à 13:29
http://fr.php.net/manual/fr/function.header.php
N'empêche qu'il pourrait être intéressant de voir un portable de tes fonctions en une classe toute bête et méchante (niark, c'est bon les classes xD)
1 juin 2006 à 12:41
Une autre façon de faire ?
oui oui oui oui... C'est celle que j'utilise (presque)
<?php
// La racine du site est :
define('ROOT', $_SERVER['DOCUMENT_ROOT']§.'/www/');
// Définition des différents droits d'acces existant
$i=0;
define('ACCES_GUEST',1<<$i++);
define('ACCES_LOGGED',1<<$i++);
define('ACCES_ADMIN',1<<$i++);
define('ACCES_SUPER',1<<$i++);
define('ACCES_ALL',0xff);
// Définition des droits d'acces aux page du site
$GLOBALS['ACCES'] = array();
$GLOBALS['ACCES'][ROOT.'index.php'] = ACCES_ALL;
$GLOBALS['ACCES'][ROOT.'connexion.php'] = ACCES_ALL;
$GLOBALS['ACCES'][ROOT.'compte_perso.php'] = ACCES_LOGGED;
// ... Ainsi pour toutes les pages du scripts
// C'est la technique dite de la liste blanche !
// Le script à inlcure est présumé coupable et doit prouver sa sympathie
// Puis une fonction pour inclusion sécurisée
function is_allow($include_this) {
$user_acces_rights = /* On chope les droits de l'utilisateur */;
// Adresse ABSOLUE et vérifie EXISTANCE du fichier ... C'est déjà pas mal !
$include_this = realpath($include_this);
// Pas le droit d'inclure :
if( $include_this===false
or !isset($GLOBALS['ACCES'][$include_this])
or !($GLOBALS['ACCES'][$include_this]&$user_acces_right) ) return false;
// OK ? cool...
return true;
}
// A utiliser ainsi pour les inclusions :
if( is_allow( $page ) ) {
include( $page );
}
// Ou ainsi pour protéger les accès aux pages :
$request = parse_url( $_SERVER['REQUEST_URI'] );
if( !is_allow($request['path']) ) {
header('HTTP/1.0 403 Not Allowed');
header('Location: /erreur_403.php');
exit;
}
?>
Il y'a bien sûr milles améliorations possibles...
A+
1 juin 2006 à 11:09
exemple si j'ai un fichier ajout_news.php dans un dossier admin proteger via .htaccess
www.monsite.com/admin/ajout_news.php <== le htaccess fonctionnera
par contre si on utilise l'URLRewriting
www.monsite.com/index.php?fichier=admin/ajout_news.php
le .htaccess ne fonctionnera pas
donc faire attention a sa...
1 juin 2006 à 11:09
1 juin 2006 à 11:01
mais metre la condition suivante
if (file_exists($page))
sufit amplement vu que si la page est externe au serveur il ne l'acceptera pas...
pour que sa fonctionne le pirate doit dabord uploader sa page virus.php sur le serveur donc si il n'y a pas de system d'upload mal protegre c'est bon...
1 juin 2006 à 10:56
1 juin 2006 à 10:50
Après, j'inclus le bon fichier par le switch, et je lui lance la fonction appropriée, qui a été définie dans le fichier inclus.
Avec ca, il est déjà impossible de voir de l'html qui ressort dun fichier inclus, et ensuite la faille include n'existe pas ;-)
Le .htaccess en plus, faudrait que j'y pense, mais comme je fonctionne sur des serveurs sur lesquels on ne peut inclure par défaut des fichiers qui ne nous appartiennent pas, je pense que j'ai pas de soucis de ce côté là ;-)
[quote=Malalam]UN admin n'est autre qu'un esclave des désirs des utilisateurs du site qu'il administre/quote
Magnifique phrase, et je susi dévis pour qu'on laisse les commentaires et que killermaster update le code en fonction des commentaires ;-)
1 juin 2006 à 10:34
pour peu qu'apache soit lancé comme un porki.. et que je me trompe pas dans la commande..
1 juin 2006 à 10:13
Ex de fichier inclus:
<?php
echo '<?php unlink("index.php") ?>';
?>
Sans le tester, je pense que ca devrait supprimer le fichier index.php du dossier courrant sur ton serveur...
Sinon, je suis interessé sur la raison...
Brice
1 juin 2006 à 10:08
Puisque le débat porte sur les différentes techniques pour parrer la faille include, je vais vous donner celle que j'utilise, pour que vous puissiez me dire ce que vous en pensez (et l'utiliser si elle vous plait, bien sur...).
J'ai un fichier de constantes:
<?php
define('INDEX','index.php');
define('CHAPITRE1','chapitres/chapitre1.php');
...
?>
J'inclus systématiquement ce fichier et je teste avant d'inclure la page demandée:
if (defined($page))
include(constant($page));
else
include('fichiererreur.php');
En fait ca me permet d'être certain que seules les pages autorisées sont incluses, de ne pas donner trop facilement l'architecture du site (pas de ?page=architecture/de/mon/site/page.php dans l'adresse) et tout simplement d'avoir des adresses plus jolies et intuitives, avec des constantes.
Brice
1 juin 2006 à 03:27
pour améliorer la faille, je vais pas avoir le temps désolé
@++
1 juin 2006 à 02:07
$page = $_GET["page"];
$page = str_replace(Array("..","."),"",$page);
if(files_exist("includes/".$page))
{
include("includes/".$page);
}
C'est juste pour éviter que tu puisse includer les fichiers .htpasswd de tout le serveur ...
31 mai 2006 à 22:57
Dans le même genre : http://www.vulgarisation-informatique.com/failles-php.php
31 mai 2006 à 22:31
mais que les commentaires aux servent a quelque choses
( a faire évoluer le débat sur ce type de faille ^^' )
31 mai 2006 à 20:01
Oh oui, pour la peine je quote moi aussi :p
31 mai 2006 à 18:27
Ooh... comme c'est beau... lol... .. . ;o)
@ tchaOo°
31 mai 2006 à 17:38
31 mai 2006 à 17:29
Vaudrait mieux que killermaster update sa source pour tenir compte de toutes les remarques, ca serait un bon compromis non ?
31 mai 2006 à 16:42
Donc, je pense que je vais laisser ce code. Pour les commentaires.
Maintenant, si, chers membres ;-), vous êtes contre, je me plierai à vos désirs, évidemment. UN admin n'est autre qu'un esclave des désirs des utilisateurs du site qu'il administre, après tout ;-)
31 mai 2006 à 16:15
31 mai 2006 à 15:26
@ CrYpToNyM... c'est pour ça qu'il vaut mieux regrouper les pages dans un répertoire ne contenant que des page autorisées à l'affichage... .. .
@ tchaOo°
31 mai 2006 à 14:49
Et si on faisait comme ça :
$page = $_SERVER['DOCUMENT_ROOT'].'/'.$_GET['page'];
Ne serait-ce pas mieux?
31 mai 2006 à 14:36
Mais bon le script kiddie peut toujours inclure n'importe quel fichier local.
La meilleure méthode reste le switch case.
31 mai 2006 à 13:30
si on prend ton exemple on incluera du code html... donc à part des attaque css tu peux pas faire grand chose... c'est un peu plus compliqué que ça... et heureusement... .. .
l'autre faute est théorique... comme le dit Malalam le strpos ne protège pas grand chose car facilement contournable... mieux vaut mettre toutes ces pages dans un répertoire protégé par un .htaccess en deny from all et faire...
$page = (isset($_GET['page'])) ? strip_tags($_GET['page']) : 'accueil';
if(is_file('pages/'.$page.'.php'))
include('pages/'.$page.'.php');
elseif(is_file('pages/'.$page.'.html'))
include('pages/'.$page.'.html');
else
include('pages/accueil.php');
Ou alors comme dit williamprotti tu fais un switch... .. .
@ tchaOo°
31 mai 2006 à 13:05
ok merci bien j'vais revoir tout ça ;)
@++
31 mai 2006 à 13:03
Personnelement je fait comme ça:
Url = http://www.site.com/?idPage=1
code
<?php
switch($_GET['idPage'])
{
case '1':
include"page.php";
break;
default:
include"erreur.php"; // ou acceuil, ou un echo etc...
break;
}
?>
Maintenant je sais pas si c'est bien de faire comme ça....
voilà! :)
31 mai 2006 à 12:35
strpos renvoie : false s'il échoue, ou un entier s'il trouve la chaîne demandée, indiquant la position de cette chaîne dans l'autre chaîne.
Mais un offset de position dans une chaîne commence à 0, pas à 1.
Dans mon exemple: test et testeur.
testeur contient évidemment test. Et la position de 'test' dans 'testeur' est 0.
Or, en php false == 0.
Mais false !0...(comparaison typée, triple).
donc, dans mon exemple, avec false strpos (...), ça renvoie true! Puisqu'il trouve 0, et que 0 false...
Mais en faisant false === strpos (...), il trouve tjrs 0, mais 0 !== false, donc il renvoie bien false, et non true.
31 mai 2006 à 12:31
avant d'inclure $page, j'ai un test (composé de switch) qui me renvoie une valeur bien précise pour $page, après je l'inclue avec
include($page);
seulement je voulais juste empêcher l'utilisateur de mettre n'importe quoi dans cette variable..
Et je ne comprends pas pourquoi tu dis que strPos ne fonctionne pas correctement !
Tu pourrais expliquer ?? :))
@++
31 mai 2006 à 12:26
bah merci quand même !
@++
31 mai 2006 à 11:50
31 mai 2006 à 11:49
Au passage, file_exists ne renvoie jamais une valeur 0, donc pas besoin de true.
Et vive la comparaison typée, au passage, parce que :
<?php
$sSearch = 'test';
$sString = 'testeur';
if (false == strpos ($sString, $sSearch)) {
echo 'non trouvé : "testeur" ne contient pas "test"';
} else {
echo 'trouvé : "testeur" contient "test"';
}
?>
et
<?php
$sSearch = 'test';
$sString = 'testeur';
if (false === strpos ($sString, $sSearch)) {
echo 'non trouvé : "testeur" ne contient pas "test"';
} else {
echo 'trouvé : "testeur" contient "test"';
}
?>
compares donc...
31 mai 2006 à 11:43
heu, et si le mec met ?page=www.sonsite.com/virus.php ...? Ou une adresse ip ? ou même pas www? ...etc
Le mieux est de toutes façons de savoir ce que tu t'attends à recevoir...et de n'accepter que ça.
Perso, j'utilise, quand j'utilise c e système, un tableau des GET que j'attends. Et ce ne sont jamais des url...ou du moins, jamais des url absolues.
31 mai 2006 à 11:38
La variable $page n'est pas remplie automatiquement si la directive "register_globals" est à "off" sur ta distribution php.
C'est une erreur commune de croire que c'est le fonctionnement normal de PHP de remplir les variables automatiquement avec l'URL, mais ca pose un problème de sécurité tellement important que par défaut ce comportement est désormais à "off" par défaut (et chez beaucoup d'hébergeurs sur des vieilles distributions aussi).
Dites moi si je me trompe...
Perso, je teste chaque paramètre de facon exacte avant un include, ca coute pas cher et ca évite les surprises :-)