[Sécurité] Comment amélioré mon code ? [Résolu]

[Nadd]

Bonjour,
En vue de crée un espace membre pour le site de Vente que je suis en train de réalisé, j'ai crée (à l'aide d'un bouqin) un code permettant de s'identifier. J'aimerais avoir votre avis dessus, mais ce qui me préoccupe le plus est de savoir si il est sécurisé... :

[members.php]

Code :

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
48
49
50
51
52
53
54
55
 
define('IN_x0', true);
 
$x0_root_path = './';
$phpEx = 'php';
include($x0_root_path . 'kern.php');
 
session_start();
 
$template = "blue";
 
$template = new Template($phpbb_root_path . "templates/".$template."/");
 
// HEADER ---------------------------------------------------------------------
include($phpbb_root_path . 'includes/header.php');
// ----------------------------------------------------------------------------
 
// VIEWNEWS -------------------------------------------------------------------
$template->set_filenames(array(
	'members' => 'productslist.html')
);
 
if(isset($_POST['username']) && isset($_POST['userpassword'])) {
 
$username = $_POST['username'];
$userpassword = md5($_POST['userpassword']);
 
if (lookat($username,$userpassword)) {
 
$_SESSION['username'] = $username;
 
$template->pparse('members');
 
}
 
else {
 
die("Username et/ou Userpassword invalide");
}
 
}
 
// ----------------------------------------------------------------------------
 
// MENU -----------------------------------------------------------------------
$template->set_filenames(array(
	'menu' => 'menu.html')
);
 
$template->pparse('menu');
// ----------------------------------------------------------------------------
 
// FOOTER ---------------------------------------------------------------------
include($phpbb_root_path . 'includes/footer.php');
// ----------------------------------------------------------------------------

Comme vous pourrez le voir, il a certaine ressemblance avec le code de base de PHPBB2

[sessions.php]

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
 
function lookat($username,$userpassword) {
 
global $db;
 
$sql = "SELECT * FROM icb_users WHERE Username='".$username."' AND Password='$userpassword';";
 
if( !($result = $db->sql_query($sql)) )
{
	die("Erreur -> Requête SQL");
}
 
if ( $row = $db->sql_fetchrow($result) )
{
return TRUE;
}
else{
return FALSE;
}
 
}

En vous remerciant d'avance,

Nadd

[zulkifli]

$sql = "SELECT * FROM icb_users WHERE Username='".$username."' AND Password='$userpassword';";

je prense que tu devrais aller voir du coté de mysql_real_escape_string si tu te préoccupe de la sécurité.

sinon ce que je pense que tu utilise des variables intermédiaire inutile mais bon ce n'est qu'un détail

je ferai plutot

Code :

1
2
3
4
5
6
7
8
 
if(isset($_POST['username']) && isset($_POST['userpassword'])) {
 
$_POST['userpassword'] = md5($_POST['userpassword']);
 
if (lookat( $_POST['username'], $_POST['userpassword'])) {
 
$_SESSION['username'] = $_POST['username'];

[Yogui]

zulkifli : Je pense que c'est une très mauvaise idée de modifier une variable comme $_POST.
Laisse-la tranquille et enregistre une nouvelle variable, ce n'est pas pour ce que ça coûte et ça te laissera le choix d'accéder à ta variable d'origine.

[Nadd]

Mis à part ca, le code est t'il plus ou moin sécurisé ?

---

Comme ceci alors :

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
 
function lookat($username,$userpassword) {
 
global $db;
 
$username_ = mysql_real_escape_string($username);
$userpassword_ = mysql_real_escape_string($userpassword);
 
$sql = "SELECT * FROM icb_users WHERE Username='".$username_."' AND Password='".$userpassword_."';";
 
if( !($result = $db->sql_query($sql)) )
{
	die("Erreur -> Requête SQL");
}
 
if ( $row = $db->sql_fetchrow($result) )
{
return TRUE;
}
else{
return FALSE;
}
 
}

En vous remerciant d'avance,

Nadd

[zulkifli]

Kirkis, je ne sais pas si tu a raison, dans la doc il n'y a rien sur ce sujet. Pour moi $_POST est un tableau associatif comme les autre sauf qu'il est SUPER global et généré par un envoi de formulaire. C'est une variable supergloabal comme $_SESSION et celle ci, on la modifie bien nan?

[zulkifli]

attention à mysql_real_string Nadd, il faut verifier avant que les magic quotes sont activés ou pas. Je te propose cette petite fonction bien pratique tirée de la doc.

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
 
function quote_smart($value)
{
   // Stripslashes
   if (get_magic_quotes_gpc()) {
     $value = stripslashes($value);
   }
   // Protection si ce n'est pas un entier
   if (!is_numeric($value)) {
     $value = "'" . mysql_real_escape_string($value) . "'";
   }
   return $value;
}

[Nadd]

Il faut savoir que mon site est basé sur PHPBB2, il reprend le moteur de template et le fichier common.php, qui traite au sujet des magics_quote.

Code :

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
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
 
error_reporting  (E_ERROR | E_WARNING | E_PARSE);
set_magic_quotes_runtime(0);
 
// Sécurité -----------------------------------------------------------------
if (isset($HTTP_POST_VARS['GLOBALS']) || isset($HTTP_POST_FILES['GLOBALS']) || isset($HTTP_GET_VARS['GLOBALS']) || isset($HTTP_COOKIE_VARS['GLOBALS']))
{
include($x0_root_path . 'abgelehnt.php');
exit();
}
 
if (isset($HTTP_SESSION_VARS) && !is_array($HTTP_SESSION_VARS))
{
include($x0_root_path . 'abgelehnt.php');
exit();
}
 
if( !get_magic_quotes_gpc() )
{
	if( is_array($HTTP_GET_VARS) )
	{
		while( list($k, $v) = each($HTTP_GET_VARS) )
		{
			if( is_array($HTTP_GET_VARS[$k]) )
			{
				while( list($k2, $v2) = each($HTTP_GET_VARS[$k]) )
				{
					$HTTP_GET_VARS[$k][$k2] = addslashes($v2);
				}
				@reset($HTTP_GET_VARS[$k]);
			}
			else
			{
				$HTTP_GET_VARS[$k] = addslashes($v);
			}
		}
		@reset($HTTP_GET_VARS);
	}
 
	if( is_array($HTTP_POST_VARS) )
	{
		while( list($k, $v) = each($HTTP_POST_VARS) )
		{
			if( is_array($HTTP_POST_VARS[$k]) )
			{
				while( list($k2, $v2) = each($HTTP_POST_VARS[$k]) )
				{
					$HTTP_POST_VARS[$k][$k2] = addslashes($v2);
				}
				@reset($HTTP_POST_VARS[$k]);
			}
			else
			{
				$HTTP_POST_VARS[$k] = addslashes($v);
			}
		}
		@reset($HTTP_POST_VARS);
	}
 
	if( is_array($HTTP_COOKIE_VARS) )
	{
		while( list($k, $v) = each($HTTP_COOKIE_VARS) )
		{
			if( is_array($HTTP_COOKIE_VARS[$k]) )
			{
				while( list($k2, $v2) = each($HTTP_COOKIE_VARS[$k]) )
				{
					$HTTP_COOKIE_VARS[$k][$k2] = addslashes($v2);
				}
				@reset($HTTP_COOKIE_VARS[$k]);
			}
			else
			{
				$HTTP_COOKIE_VARS[$k] = addslashes($v);
			}
		}
		@reset($HTTP_COOKIE_VARS);
	}
}
// --------------------------------------------------------------------------

set_magic_quotes_runtime(0) == magic_quotes désactivée

Amicalement,

Nadd

[Yogui]

Envoyé par zulkifli

Kirkis, je ne sais pas si tu a raison, dans la doc il n'y a rien sur ce sujet. Pour moi $_POST est un tableau associatif comme les autre sauf qu'il est SUPER global et généré par un envoi de formulaire. C'est une variable supergloabal comme $_SESSION et celle ci, on la modifie bien nan?

Tu n'as pas tort, ce n'est pas interdit. Cependant, j'ai tendance à considérer $_POST et $_GET comme des sources de données à ne pas modifier. Ces tableaux sont fournis par l'utilisateur, je considère que modifier l'une de ces valeurs revient à faire comme si l'utilisateur avait envoyé autre chose, ce qui n'est pas le cas.
Bref, je me comprends, tu fais comme tu le sens mais je pense que modifier un tableau POST ou GET est une erreur de logique, tandis que SESSION est là pour ça. Cela n'a pas de rapport direct avec leur caractère superglobal, c'est simplement une manière de voir les choses.

[macbook]

On peut faire de la réassignation de valeur sur n'importe quelle variable, qu'elle soit superglobale ou local ne change rien et n'a aucune incidence sur le reste.

[Nadd]

Dois-je mettre un " session_start() " sur tout mes pages, même celle qui ne demande aucune identification, sachant aussi que je ne fais aucune destruction de la session dans aucune de mes pages.

Pour ma part je ne metterais le session_start() que sur ma page de vérification.

[Yobs]

Tu dois mettre un session_start au début de toutes les pages concernées par ta session.

[Nadd]

Merci beaucoup pour votre précieuse aide à tous. J'espère que mon code est un minimum sécurisé,

Cordialement,

Nadd.[/code]

[Nadd]

Hum, encore une dernière question,

je veux que pour se déconnecter, l'utilisateur clic sur "Se déconnecter" ce qui l'envoi sur unconnect.php que voici :

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
<?php
/***************************************************************************
 * IC-BUSINESS unconnect.php
 ***************************************************************************/
 
session_start();
 
$_SESSION['AUTH'] = FALSE;
session_destroy();
 
header("Location: index.php");
 
?>

Y'a t'il une faille de sécurité la dedans ? Je ne penses pas, mais je demande quand même

[Yobs]

Voici le code de la doc officielle pour plus de sécurité.

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
<?php
 
// Initialisation de la session.
// Si vous utilisez un autre nom
// session_name("autrenom")
session_start();
 
// Détruit toutes les variables de session
$_SESSION = array();
 
// Si vous voulez détruire complètement la session, effacez également
// le cookie de session.
// Note : cela détruira la session et pas seulement les données de session !
if (isset($_COOKIE[session_name()])) {
   setcookie(session_name(), '', time()-42000, '/');
}
 
// Finalement, on détruit la session.
session_destroy();
 
?>

[Nadd]

Merci beaucoup !