Discussion :

[dancom5]

Bonjour.

Je viens de faire la découverte de ceci : "mysql_fetch_object" et je viens tout juste de créer un script qui modifie des données dans la base de données.

Je voudrais savoir si mon script est biens sécurisé.

J'apprécierais de l'aide afin de savoir si j'ai pas oublié un élément pour sécuriser mon code.

Une note en passant, je test avec un addon de firefox pour les injections sql et les xss. Ça dit aucune faille mais les données dans la base changent quand même sur tous les ID.

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
48
 
<?php
$db = mysql_connect("localhost","root","pass") or die("cannot connected");
@mysql_select_db("origami",$db);
 
if(isset($_POST['id']) && isset($_POST['titre']) && isset($_POST['desc'])){
 
	$uid=mysql_real_escape_string($_POST["id"]);
	$utitre=mysql_real_escape_string($_POST["titre"]);
	$udesc=mysql_real_escape_string($_POST["desc"]);
 
	$requete = mysql_query("UPDATE `ori_images` SET `titre`='$utitre',`desc`='$udesc' WHERE `id`='$uid'");
 
	if($requete){
		echo("La modification à été correctement effectuée") ;
	}
	else{
		echo("La modification à échouée") ;
	}
}
 
$id=$_GET['id'];
 
$requete=mysql_query("SELECT * FROM ori_images WHERE id='$id'");
 
if($result=mysql_fetch_object($requete)){
	echo $result->id;
  ?>
 
  <h3>Edit</h3>
  <form method="POST" action="">
 
  <input type="hidden" name="id" value="<?php echo htmlspecialchars($result->$id); ?>">
 
  titre: <input name="titre" size="80" value="<?php echo htmlspecialchars($result->titre); ?>"><br><br>
  desc: <textarea name="desc"  rows="5" cols="80"><?php echo htmlspecialchars($result->desc); ?></textarea><br><br>
 
  <input type="submit" name="submit" value="update">
  </form>
 
	<?php
}
 
?>  
 
<a href="admin.php">Admin</a>
</body>
</html>

J'ai besoin d'un avis. Merci. Si j'ai une faille, j'apprécierais de la combler.

Au lieux de créer deux fichier pour modifier les données, j'ai fait pour que ça soit dans le même fichier : La modification et le résultat.

Normalement, j'aurais le formulaire dans un fichier et le traitement dans le deuxième. Mais je trouvais que ça irait mieux en un seul fichier.

[Benjamin Delespierre]

L'extension mysql n'est plus supportée par PHP, je te recommande de passer dès que possible à mysqli ou PDO (qui supportent également le fetch object).

Ton code m'a l'air bon à deux détails près:
- l'usage de mysql_real_escape_string peut être remplacé avantageusement par des requêtes préparées avec des placeholder. Voir PDOStatement pour un exemple.
- Tu ne contrôle à aucun moment l'exécution de tes requêtes, si l'une d'elle échoue, ça va lancer des warnings et potentiellement crasher ce qui suit.
Je te recommande de wrapper toutes tes exécutions de requête d'une manière générale dans des if pour vérifier le status.

Normalement, j'aurais le formulaire dans un fichier et le traitement dans le deuxième. Mais je trouvais que ça irait mieux en un seul fichier.

Normalement, des fonctionnalités Métier et Modèle n'ont rien à faire dans une vue mais bon...

[sabotage]

Dans ta requête ligne 23, la valeur n'est pas protégée.
De plus quand tu mets une valeur numérique dans une requêtes, utilise intval() et non mysql_real_escape_string() et ne mets pas de guillemets.

[dancom5]

Je regarde pour mysqli puisque PDO n'est pas accessible pour moi par mon serveur et non plus sur mon serveur local de test.

Je regarde en détails ce que vous m'avez écrit. Si je pouvais trouver un tuto sur mysqli pour débuter, ça me serait intéressant.

J'ai revu mon code pour le rendre plus sécure. Mais, je fais aussi une version mysqli

version mysql:

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
48
49
50
51
52
53
54
55
<?php
$db = mysql_connect("localhost","...","...") or die("cannot connected");
@mysql_select_db("origami",$db);
 
function JsDisabler($interdir){
		$interdir=preg_replace('#[<>&;=]#i',"",$interdir);
	return $interdir;
}
 
if(isset($_POST['id']) && isset($_POST['titre']) && isset($_POST['desc'])){
 
	$uid=intval($_GET['id']);
 
	$utitre=mysql_real_escape_string(JsDisabler($_POST["titre"]));
	$udesc=mysql_real_escape_string(JsDisabler($_POST["desc"]));
 
	$requete = mysql_query("UPDATE `ori_images` SET `titre`='$utitre',`desc`='$udesc' WHERE `id` ='$uid'");
 
	if($requete){
		echo("La modification à été correctement effectuée<br><br>") ;
	}
	else{
		echo("La modification à échouée<br><br>") ;
	}
}
 
if(isset($_GET['id'])){
 
	$id=intval($_GET['id']);
 
	$requete=mysql_query("SELECT * FROM ori_images WHERE id='$id'");
 
	if($result=mysql_fetch_object($requete)){
		echo $result->id.'<br>';
		?>
 
		<h3>Edit</h3>
		<form method="POST" action="">
 
		<input type="hidden" name="id" value="<?php echo $id; ?>">
 
		titre:<br><input name="titre" size="80" value="<?php echo $result->titre; ?>"><br><br>
		desc:<br><textarea name="desc"  rows="5" cols="80"><?php echo $result->desc; ?></textarea><br><br>
 
		<input type="submit" name="submit" value="update">
		</form>
 
		<?php
	}
}
 
?>  
<a href="admin.php">Admin</a>
</body>
</html>

Question sur "wrapper" ?

Pour les xss, j'interdit les balises html <> avec preg_remplace qui me semble efficace. htmlentities et l'autre, me pose un probleme.

Merci Benjamin pour l'aide accordé!

Merci sabotage, j'ai pris note et corrigé mon code.

..je te recommande de passer dès que possible à mysqli ou PDO..

Ton code m'a l'air bon à deux détails près:
- l'usage de mysql_real_escape_string peut être remplacé avantageusement par des requêtes préparées avec des placeholder. Voir PDOStatement pour un exemple.
- Tu ne contrôle à aucun moment l'exécution de tes requêtes, si l'une d'elle échoue, ça va lancer des warnings et potentiellement crasher ce qui suit.
Je te recommande de wrapper toutes tes exécutions de requête d'une manière générale dans des if pour vérifier le status.


Normalement, des fonctionnalités Métier et Modèle n'ont rien à faire dans une vue mais bon...

[stealth35]

J
Pour les xss, j'interdit les balises html <> avec preg_remplace qui me semble efficace. htmlentities et l'autre, me pose un probleme.

strip_tags serai plus simple

[Benjamin Delespierre]

Par "wrapper" (embalage en anglais) je voulais dire mets tes exécutions de requêtes dans des if pour contrôller leur exécution.

[dancom5]

Merci. J'utilise maintenant Mysqli, je délaisse peu à peu Mysql et le PDO, je ne peux pas l'utiliser.

Ça prends du temps pour assimiler.