[Sécurité] Structures de contrôle et array dans un array

**moonblood** · 08/03/2007, 18h11

Bonjour à tous !
Je suis nouveau, et pas très doué alors si je met ce message au mauvais endroit ou dis des bêtises, soyez gentil de pas me taper dessus et je modifierai....

J'ai réalisé un site dont la navigation se passe comme ceci :
index2.php?page=page01
index2.php?page=page02
[...]

et dont le script central est :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
 
<?php
	if ( isset($_GET['page']) ) $page= 'page01';
	else $page= $_GET['page'];
	switch($_GET['page'])
                 {
		    case 'mapremierepage': include ('page01.txt');break;
                    case 'mapremierepage': include ('page02.txt');break;
                    // [...]
                    default: include('mapremierepage.txt');break;
                  }
?>

Jusque là tout va bien, mon switch empechant que n'importe quelle page soit affichée, et si qqun tapait n'importe quoi, il reviendrait par défaut à la première page.

Etant un brin parano, j'ai rajouté au tout début de mon script un header location de sécurité comme ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
<?php
$pages_autorisees = array(
'page01',
'page02',
'page03',
);
if (!in_array($_GET[page], $pages_autorisees))
{
header("Location: index.php");
}
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html>
<head>
[...]

Jusque là tout va encore bien.
-Première question : D'un point de vue sécurité mon header location se justifie-t-il ou finalement j'aurai pu tout aussi bien le supprimer ?

Seulement développant mon site j'ai aujourd'hui besoin de rajouter d'autres variables et d'avoir une adresse du style : "site.com/index2.php?page=page01&name=var1"
-Deuxième question D'un point de vue sécurité, est-ce grave si les visiteurs peuvent taper ce qu'ils veulent dans les variables ?

Etant toujours parano, j'ai modifié mon script :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
<?php
$pages_autorisees = array(
'page01',
'page02',
'page03',
);
$names_autorises = array(
'nom01',
'nom02',
);
if (!in_array($_GET[page], $pages_autorisees))
{
header("Location: index.php");
}
elseif ( (isset($_GET['name'])) && (!in_array($_GET[name], $names_autorises)) )
{
header("Location: index.php");
}
?>

Donc là je vérifie que ma seconde variable name existe, et si elle existe, mais qu'elle est differente de ma liste autorisée, on revient au point de départ. cela fonctionne donc dans ce cadre là :
"site.com/index.php?page=page01&name=variableinconnue"

Cela retourne bien effectivement automatiquement à la page d'accueil index.php.

Mais le problème c'est que si l'on écrit cela :
"site.com/index.php?page=page01&nameexample=variableinconnue"
ou même "site.com/index.php?page=page01&XXXX"
Le site ne retourne pas a l'index.php mais reste sur page=var tout court.

Voilà ce n'est peut être pas un vrai problème que les gens puissent mettre ce qu'ils souhaitent dans XXXX mais cela me chiffonne, à mon avis vous aurez déjà répondu à ma seconde question.

Une piste que l'on m'a donné est celle-ci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
<?php
    $pages_autorisees = array (
        'page01' => array ( 'var01', 'var2' ) ,
        'page02' => '',
        'page03' => array ( 'var03', 'var04' )
    ) ;
    if ( // ????  )
        {
    header("Location: index.php");
        }
?>

Il s'agirait donc de mettre un array des variables autorisées à l'intérieur de l'array des pages autorisés.
Je l'ai fais et ça fonctionne, on peut même voir les résultats du tableau à l'aide d'un print_r($pages_autorisees);
-Troisième question Que faut-il écrire dans le if comme condition pour qu'il vérifie à la fois ce qu'il y a dans page et dans name et qu'il ramène au départ si les données insérées ne sont pas contenues dans le tableau des données autorisées ?

Si quelqu'un avait la connaissance et la joie de m'aider ce serait vraiment sympa

Si vous trouvez une autre solution, c'est tout bon aussi !
Merci beaucoup d'avance !

**nabab** · 09/03/2007, 02h02

C'est vraiment pas top de faire tes appels de page comme ca! D'autant que si tes pages sont limitees (tu les enumeres dans ton switch), et qu'elles font appel a des variables differentes, cela ne presente aucun interet: autant utiliser des pages differentes (page1.php, page2.php...).
Ne pas appeler un fichier directement par une variable comme tu le fais ici, et traiter les variables que tu recois (par un addslashes par exemple) est bien plus efficace qu'enumerer les possibilites.
Regarde quand meme les topics sur la securite et ta config php (phpinfo) pour voir si ton serveur court des risques.

**moonblood** · 09/03/2007, 11h12

Envoyé par nabab

Ne pas appeler un fichier directement par une variable comme tu le fais ici, et traiter les variables que tu recois (par un addslashes par exemple) est bien plus efficace qu'enumerer les possibilites.

heu oui, je veux bien, mais pourrais tu être plus précis ?
(je suis un peu une quiche et je débute)

Envoyé par nabab

Regarde quand meme les topics sur la securite

Je viens de le faire, et j'ai rien trouvé de vraiment gênant, je posais quand même la question, sait on jamais ...

**Herode** · 09/03/2007, 11h48

J'ai du rater un détail, parce que ça me semble bien compliqué comme système, donc fragile. Quelques questions en vrac :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
if ( isset($_GET['page']) ) $page= 'page01';
	else $page= $_GET['page'];

Donc si $_GET['page'] est renseigné, $page est initialisé à 'page01' et s'il n'est pas renseigné, $page est initialisé à ?????
A quoi sert $page ????

Jusque là tout va bien, mon switch empechant que n'importe quelle page soit affichée, et si qqun tapait n'importe quoi, il reviendrait par défaut à la première page.

Si le switch filtre les mauvaises saisies, pourquoi ajouter des contrôles en doublon ?????

Je dirai même plus : si ton switch fait son boulot d'aiguillage, ce qui est une bonne idée, alors avoir des contrôles en doublon
* au mieux ne sert à rien,
* au pire sera une source d'erreurs.
D'une manière générale en informatique, toute opération est une source d'erreur. A fortiori les doublons

Mon avis serait donc : concentre-toi sur le switch et fais le parsing de ton url comme il faut en traitant les cas connus (arguments attendus), les autres (erreurs d'url ou acte hostile) allant sur une page par défaut (index, page d'erreur, etc...). Et fais-le en un seul endroit.

**moonblood** · 09/03/2007, 19h45

Envoyé par Herode

J'ai du rater un détail, parce que ça me semble bien compliqué comme système, donc fragile. Quelques questions en vrac :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
if ( isset($_GET['page']) ) $page= 'page01';
	else $page= $_GET['page'];

Donc si $_GET['page'] est renseigné, $page est initialisé à 'page01' et s'il n'est pas renseigné, $page est initialisé à ?????
A quoi sert $page ????

Tu as raison !
Je viens de retoucher mon code et le code de ma page principale (index2.php)
ressemble maintenant à ça :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
[...]
 
         <?php
			switch($page)
            {
		    case 'page01': include ('menu-page01.txt');break;
		    case 'page02': include ('menu-page02.txt');break;
		    default: include('menu-page01.txt');break;
            }
          ?>
 </div><!-- end menu -->
 
	  <div id="centre">
 
	  	    <?php
                     include ("$page.txt");
                     ?>
 </div><!-- end page centrale -->
 
[...]

Envoyé par Herode

Mon avis serait donc : concentre-toi sur le switch et fais le parsing de ton url comme il faut en traitant les cas connus (arguments attendus), les autres (erreurs d'url ou acte hostile)

Ben j'ai simplifié le switch, mais le fait est que j'ai pas trouvé de meilleur moyen de redirection que mon header("Location en début de page.

D'ailleurs, je viens de résoudre mon problème grâce à Ereg !

Bon c'est pas ce qu'il y a de plus optimisé, donc je vous livre le code le plus clairement possible avec commentaires, je rappelle que les saisies que je veux autorisées sont de ce style index2.php?page=page01 ou index2.php?page=page01&name=jean :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
 
<?php
$page = $_GET["page"];
$name = $_GET["name"];
$url = $_SERVER['REQUEST_URI'];
$pages_autorisees = array (
     'page01',
     'page02',
     );
$names_autorises_default= array(
     '',
      );
$names_autorises_page01 = array(
     '&name=jean',
      );
$names_autorises_page02 = array(
     '&name=emile',
     '&name=pierre',
      );
 
switch($page) // le switch des variables autorisées selon la page appelée
  {
  case 'page01': $names_autorises = $names_autorises_page01 ; break; 
  case 'page02': $names_autorises = $names_autorises_page02 ; break; 
  default: $names_autorises = $names_autorises_default ;break;
  }
 
ereg("/index2\.php\?page=$page(.+)", $url, $regs);  // ereg() recherche la séquence de caractères qui suit ma variable $page
 
if ( !isset($page) or !in_array($page, $pages_autorisees)) 
{
   header("Location: index.php");  // print " ATTENTION la variable \$page n'existe pas OU alors la valeur de celle-ci n'est pas valable !";   
}
elseif ( ($regs[1] != "") && (!in_array($regs[1], $names_autorises)) ) 
{
   header("Location: index.php"); // print " ATTENTION Un texte/variable suit la variable \$page=$page mais ne fait pas parti des variables autorisées !";  // 
}
?>

Bon voilà, ça doit sûrement vous paraître moyen moyen mais je suis une quiche, je le répète, alors si vous avez mieux à me proposer, je prend ! ^^

(pour le moment j'en suis à empecher et à rediriger toute saisie différente de celles autorisées, mais j'ai 1 variable !
comment survivre le jour où j'aurai un adressage de ce type :
index2.php?page=page01&name=marc&sexe=masculin&age=32&ville=marseille .... snif !

**Herode** · 09/03/2007, 20h30

(pour le moment j'en suis à empecher et à rediriger toute saisie différente de celles autorisées, mais j'ai 1 variable !
comment survivre le jour où j'aurai un adressage de ce type :
index2.php?page=page01&name=marc&sexe=masculin&age=32&ville=marseille .... snif !

C'est bien pour ça que je dis que ce code est fragile (et en plus difficile à lire car compliqué

). Or, il est impératif de penser à l'évolution et à la maintenance du code dès le début, sinon tu vas te retrouver avec des sacs de nouilles assez pénibles à manipuler.

Pourquoi ne pas essayer quelque chose de plus linéaire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
if ( ! array_key_exists( 'page', $_GET ) ) {
	header( 'Location: index.php' ); 
	return;
} 
 
switch ( $_GET[ 'page' ] ) {
	case 'page01' : return processPage01();
	case 'page02' : return processPage02();
	default : 
		header( 'Location: index.php' ); 
		return;
}
 
 
function processPage01()
{
	if ( ! array_key_exists( 'name', $_GET ) ) {
		// traiter l'erreur 
		return;
	} 
 
	switch ( $_GET[ 'name' ] ) {
		case 'jean' : // traitements du cas
		default : // // traitements erreur
	}
 
	/*etc*/
}
 
 
function processPage02()
{ 
	if ( ! array_key_exists( 'name', $_GET ) ) {
		// traiter l'erreur 
		return;
	} 
 
	switch ( $_GET[ 'name' ] ) {
		case 'emile' : // traitements du cas
		case 'pierre' : // traitements du cas
		default : // // traitements erreur
	}
 
	/*etc*/
}
 
etc...

Je te l'accorde, pour le moment ça ne fait pas beaucoup moins de lignes que ton code, mais c'est plus simple et plus homogène. Chaque fonction traite les autorisations de la page qu'elle a en charge. Ca isole les traitements pour chaque type de page, donc ça facilite la maintenance et l'évolution. Tu peux te passer des tableaux du début puisqu'ils sont implicites dans les switch que de toutes façons tu aurais eu à faire pour aiguiller les traitements si aiguillage il doit y avoir. Tu n'as pas besoin de tripoter $_REQUEST, $_GET suffit. Tu n'as pas besoin de maintenir une expression régulière (preg_match ira plus vite, d'ailleurs) qui risque de se complexifier très vite.

NB : avec ce système de header() qui rappelle les pages index.php, ce ne sera pas forcément simple d'afficher des messages d'erreur

[Sécurité] Structures de contrôle et array dans un array

Langage PHP

Vue hybride

Discussions similaires

Partager

Partager