Faille include

if ( eregi("../",$dossier) || empty($dossier) || substr($dossier,0,1) = > '/' ) {

Tout simplement, on déclare qu'il s'agit d'une tentative de hack si le
dossier est "../" ou "" (rien, donc le répertoire courant) ou "/"
(encore le répertoire courant).

En fait, je présume que c'est une condition pour lister un sous
répertoire contenant des fichiers à télécharger/afficher, donc en aucun
cas tu n'autorises l'accès à la liste des fichiers du répertoire parent
ou du répertoire courant.

--
Astuces pour webmasters: http://www.crazycat.info
Tchat francophone: http://www.crazy-irc.net

J'ai trouvé ce bout de code, il parait parfaitement adapté, mais je ne le
comprend pas.

Affirmation curieuse. ;-)

Difficile de savoir si un code est adapté (et plus exactement à quoi il
est adapté) quand on ne comprend pas ce qu'il fait !

if ( eregi("../",$dossier) || empty($dossier) || substr($dossier,0,1) = > '/' ) {

eregi("../",$dossier)
=> Teste si le chemin contient « ../ », ce qui effectivement est une
tentative de crack assez commune pour remonter plus haut dans
l'arborescence que ce à quoi on a droit.

<petit éclat de rire au passage>
Le test avec ereg*i* permet de trouver les « ../ » en minuscules
aussi bien qu'en majuscules. Je cherche toujours l'intérêt.
</>

Bon, cela dit, un strstr($dossier,'../') serait plus facile à lire,
outre qu'il économise quelques pouillèmes de microsecondes.

empty($dossier)
=> Teste si le chemin est vide, c'est-à-dire le répertoire courant.
Bon, si c'était vraiment dangereux, il suffirait à un éventuel
cracker de mettre "./" à la place, ce qui n'est pas testé.

substr($dossier,0,1) == '/'
=> Teste si le chemin part de la racine (/) au lieu de partir du
dossier courant. Tentative de crack assez commune aussi.

Cordialement,
--
Olivier Miakinen

if ( eregi("../",$dossier) || empty($dossier) || substr($dossier,0,1)
= >> '/' ) {

empty($dossier)
=> Teste si le chemin est vide, c'est-à-dire le répertoire courant.
Bon, si c'était vraiment dangereux, il suffirait à un éventuel
cracker de mettre "./" à la place, ce qui n'est pas testé.

Et c'est pour cette raison que ces tests sont dangereux. Il est facile de
prouver que ca permet d'eviter certains hacks, il est vraiment difficile de
prouver que ca les évite tous (il y a des chances que sur un windows au
moins, '..' comme dossier remonte aussi l'arborescence par exemple).

Un truc comme ca me semble vachement plus sur (jusqu'à ce que quelqu'un me
montre une faille évidemment :))

function IsPrefix($long, $prefix)
{
return substr($long, 0, strlen($prefix)) === $prefix;
}

function IsSubDir($sub, $base)
{
return IsPrefix(
realpath($sub ).DIRECTORY_SEPARATOR,
realpath($base).DIRECTORY_SEPARATOR);
}


if (!IsSubDir($dossier, '.')) {
die("tentative de hack detectée");
}

eregi("../",$dossier)
=> Teste si le chemin contient « ../ », ce qui effectivement est une
tentative de crack assez commune pour remonter plus haut dans
l'arborescence que ce à quoi on a droit.

empty($dossier)
=> Teste si le chemin est vide, c'est-à-dire le répertoire courant.
Bon, si c'était vraiment dangereux, il suffirait à un éventuel
cracker de mettre "./" à la place, ce qui n'est pas testé.

substr($dossier,0,1) == '/'
=> Teste si le chemin part de la racine (/) au lieu de partir du
dossier courant. Tentative de crack assez commune aussi.

Bonjour,

Et ce qui suit vous paraît-il correct ou dangereux ?

if(isset($_REQUEST['article'])) {
$article = $_REQUEST['article'];
$pos0 = strpos($article, './'); // "../" contient "./"
$pos1 = strpos($article, '/');
if ($pos0 === false && $pos1 !== 0) {
$articlehtm = "$article.htm";
if(file_exists($articlehtm)) {
$PageAffichee = $articlehtm;
} else {
$PageAffichee = 'accueil.htm';
}
}
else {
$PageAffichee = 'accueil.htm';
}
}
else {
$PageAffichee = 'accueil.htm';
}

Merci de vos avis, cordialement,

Cyriloch

Et ce qui suit vous paraît-il correct ou dangereux ?

[snip]

Merci de vos avis, cordialement,

Je veux bien analyser le code pour savoir ce qu'il fait, mais en soi
c'est surtout l'usage qui est fait d'un code qui peut être ou non
dangereux. Le code de Bon.Plan, appliqué à une chaîne qu'il aurait
remplie lui-même et/ou dont le résultat sert juste à faire un
« echo $dossier; », n'est *pas* dangereux. Le même code appliqué à
une variable provenant de l'extérieur et dont le résultat servirait
à faire un include() ou un exec() *est* dangereux.

Dans ton cas, on sait déjà que la chaîne vient de l'extérieur
($_REQUEST[]). Si tu t'en sers pour faire un include(), *c'est*
*probablement* dangereux. Si tu t'en sers pour faire un readfile(),
il faut voir. Si tu le stockes dans une base de données, il faut
aussi voir ce que tu en fais après... et ce que tu en feras dans
deux ans si jamais tu avais oublié que cette donnée venait de l'extérieur.

Dans tous les cas, un contrôle de sécurité sur un champ ne doit pas
se faire par la négative, c'est-à-dire en vérifiant que le champ ne
contient *pas* certaines séquences de caractères, mais de façon active
en vérifiant que le champ ne contient *que* certaines séquences dûment
réfléchies.

Je veux bien analyser le code pour savoir ce qu'il fait, mais en soi
c'est surtout l'usage qui est fait d'un code qui peut être ou non
dangereux.
...
Dans tous les cas, un contrôle de sécurité sur un champ ne doit pas
se faire par la négative, c'est-à-dire en vérifiant que le champ ne
contient *pas* certaines séquences de caractères, mais de façon active
en vérifiant que le champ ne contient *que* certaines séquences dûment
réfléchies.

Aussi te remerciai-je pour cette explication claire et détaillée... Plus
j'avance et plus le chemin semble long : mais c'est bien là ce qui
fait l'intérêt !

Cordialement,

Cyriloch