Forum d'entraide PHPFrance

Bonjour je voudrait savoir si j'ai une faille upload sur mon script ou pas merci beaucoup!

<?php

require_once('include/mysql.php');

if (isset($_POST['submit_top_match']))
{
	if(isset($_POST['name_adversaire'], $_POST['name_games'], $_POST['jours_top_match'], $_POST['mois_top_match'], $_POST['annees_top_match'], $_POST['heures_du_match'], $_POST['minutes_du_match'], $_POST['ladder_du_match'], $_FILES['logo_adversaire']))
	{
			$name_adversaire = trim(addslashes($_POST['name_adversaire']));
			$name_games = trim(addslashes($_POST['name_games']));
			$jours_top_match = trim(addslashes($_POST['jours_top_match']));
			$mois_top_match = trim(addslashes($_POST['mois_top_match']));
			$annees_top_match = trim(addslashes($_POST['annees_top_match']));
			$heures_du_match = trim(addslashes($_POST['heures_du_match']));
			$minutes_du_match = trim(addslashes($_POST['minutes_du_match']));
			$ladder_du_match = trim(addslashes($_POST['ladder_du_match']));
			
			$taille_maxi_image = '300000';
			$dossier_image = 'img/base/';
			$extensions_image = array('.png');
			$taille_image = filesize($_FILES['logo_adversaire']['tmp_name']);
			$extension_image = strrchr($_FILES['logo_adversaire']['name'], '.');
			$logo_adversaire = basename($_FILES['logo_adversaire']['name']);
		
		if(empty($name_adversaire))
		{
			echo '<h3><font color="#ff0000"><b>Le champs du nom de l\'équipe adversaire est vide</b></h3></font>';
			
		} 
		else if(empty($name_games))
		{
			echo '<h3><font color="#ff0000"><b>Le champ pour le nom du jeux est vide</b></h3></font>';
		} 
		else if(empty($jours_top_match))
		{
			echo '<h3><font color="#ff0000"><b>Le jour du top match est vide</b></h3></font>';
		}
		else if(empty($mois_top_match))
		{
			echo '<h3><font color="#ff0000"><b>le champ moi du top match est vide</b></h3></font>';
		}
		else if(empty($annees_top_match))
		{
			echo '<h3><font color="#ff0000"><b>le champ année du top match est vide</b></h3></font>';
		} 
		else if(empty($heures_du_match))
		{
			echo '<h3><font color="#ff0000"><b>le champ heure du top match est vide</b></h3></font>';	
		}
		else if(empty($minutes_du_match))
		{
			echo '<h3><font color="#ff0000"><b>le champ minute du top match est vide</b></h3></font>';
		}
		else if(empty($ladder_du_match))
		{
			echo '<h3><font color="#ff0000"><b>le champ ladder du top match est vide</b></h3></font>';
		}
		else if(empty($ladder_du_match))
		{
			echo '<h3><font color="#ff0000"><b>le logo du top match est vide</b></h3></font>';
		}
		else
		{
			$logo_adversaire = preg_replace('/([^.a-z0-9]+)/i', '-', $logo_adversaire);
			$logo_adversaire = strtr($logo_adversaire,'ÀÁÂÃÄÅÇÈÉÊËÌÍÎÏÒÓÔÕÖÙÚÛÜÝàáâãäåçèéêëìíîïðòóôõöùúûüýÿ','AAAAAACEEEEIIIIOOOOOUUUUYaaaaaaceeeeiiiioooooouuuuyy'); 
			
			// je vérifie que l'images n'existe déjà pas.
			if (file_exists($dossier_image. $logo_adversaire))
			{
				echo '<h3><font color="#ff0000"><b>le fichier '.htmlentities($logo_adversaire).' existe déjà</b></h3></font>';
			}
			else
			{
				if(!in_array($extension_image, $extensions_image))
				{	
					echo '<h3><font color="#ff0000"><b>Merci de upload une image en format png</b></h3></font>';
				}
				else
				{
					if($taille_image > $taille_maxi_image)
					{
						echo 'l\'image à une taille trop grosse';
					}
					if(move_uploaded_file($_FILES['logo_adversaire']['tmp_name'], $dossier_image . $logo_adversaire)) 
					{
						echo 'Le logo de la team adversaire à bien été upload';
					}
					else
					{
						echo '<h3><font color="#ff0000"><b>Echec : Le logo à pas été upload</b></h3></font>';
					}	
				
						$insertion_top_match = $bdd->prepare('INSERT INTO `top_match` (`name_adversaire`, `name_games`, `jours_top_match`, `mois_top_match`, `annees_top_match`, `heures_du_match`, `minutes_du_match`, `ladder_du_match`, `logo_adversaire`)
															VALUES(:name_adversaire, :name_games, :jours_top_match, :mois_top_match, :annees_top_match, :heures_du_match, :minutes_du_match, :ladder_du_match, :logo_adversaire)');
						$insertion_top_match->bindValue(':name_adversaire', $name_adversaire, PDO::PARAM_STR);
						$insertion_top_match->bindValue(':name_games', $name_games, PDO::PARAM_STR);
						$insertion_top_match->bindValue(':jours_top_match', $jours_top_match, PDO::PARAM_STR);
						$insertion_top_match->bindValue(':mois_top_match', $mois_top_match, PDO::PARAM_STR);
						$insertion_top_match->bindValue(':annees_top_match', $annees_top_match, PDO::PARAM_STR);
						$insertion_top_match->bindValue(':heures_du_match', $heures_du_match, PDO::PARAM_STR);
						$insertion_top_match->bindValue(':minutes_du_match', $minutes_du_match, PDO::PARAM_STR);
						$insertion_top_match->bindValue(':ladder_du_match', $ladder_du_match, PDO::PARAM_STR);
						$insertion_top_match->bindValue(':logo_adversaire', $logo_adversaire, PDO::PARAM_STR);
							
						$insertion_top_match->execute();
					
						$insertion_top_match->closeCursor();
							
					if($insertion_top_match)
					{
						echo '<br />Le top match à bien été ajouter';
					}
					else
					{
						echo '<h3><font color="#ff0000"><b>Le top match à pas plus être ajouter</b></h3></font>';
					}	
				}
			}
		}
	}
}
?>

Bah j'ai pas regardé dans le détail mais cela semble assez cohérent.

IMPORTANT : Niveau sécurité avant le move_uploaded_file, il faudrait plutôt faire un getimagesize sur l'image pour renforcer le contrôle, c'est un bien meilleur test que le simple contrôle de l'extension. D'ailleurs par la même occasion tu pourrais stocker la taille de ton image dans la bdd pour avoir un meilleur contrôle de l'affichage.

Niveau gestion des erreurs de l'upload t'as fait le service minimum, il en manque beaucoup (upload_max_filesize, post_max_size) ce qui fait que le téléchargement pourrait ne pas se faire et/ou retourner une page blanche sans avertissement pour le visiteur. Tu peux regarder ce tuto pour une gestion plus complète des erreurs ou utiliser cette classe pour une mise en place rapide incluant de nombreuses options.

Niveau paramètre, 300000 octet ça fait moins de 30 ko; c'est un peu juste pour une image ou alors ce ne seront que de vignettes.

Niveau code il faut faire le remplacement :

$logo_adversaire = strtr($logo_adversaire,'ÀÁÂÃÄÅÇÈÉÊËÌÍÎÏÒÓÔÕÖÙÚÛÜÝàáâãäåçèéêëìíîïðòóôõöùúûüýÿ','AAAAAACEEEEIIIIOOOOOUUUUYaaaaaaceeeeiiiioooooouuuuyy');

AVANT

$logo_adversaire = preg_replace('/([^.a-z0-9]+)/i', '-', $logo_adversaire);

Dans ton code c'est l'inverse et ton tableau de remplacement ne sert plus à rien. Utilises aussi de préférence str_replace plutôt que strtr car cette fonction n'est pas compatible utf-8.

Et tel que as conçu ton code, le fichier sera re téléchargé (ou re proposé en téléchargement) si le visiteur rafraichi la page. Pour éviter cela, Il faudrait par exemple faire suivre l'upload d'un reload en ayant préalablement enregistré les erreurs dans une variable de session (pour affichage après le reload).

En bref tu as peu de chose à ajouter au niveau sécurité pure par contre niveau ergonomie/gestion des erreurs y'a encore pas mal de travail à faire.

Rien à voir avec la question, mais si tu fais un bindValue avec une requete préparé, alors le addslaches me semble inutile, tu dois même te retrouvé avec de joli \' sur tes pages ?

(PS: c'est quoi une faille d'upload ?)

(PS: c'est quoi une faille d'upload ?)

Une faille majeure serait de ne faire aucun contrôle ni sur la nature ni sur l'extension du fichier uploadé par exemple (si le module est accessible à tout public).

Ouais pour confirmer ce que dit AB, le getimagesize important car très facile de mettre du code et de l'enregistrer au format image. Après se débrouiller pour le faire exécuter au serveur sera une autre affaire, mais comme ça ta déjà ça de filtré.

D'ailleurs par la même occasion tu pourrais stocker la taille de ton image dans la bdd pour avoir un meilleur contrôle de l'affichage

.

Tu peux m'expliquer ? Ça m’intéresse. Comment ça avoir un meilleur contrôle ? Ça optimise ? Ça fait quoi ?

merci à tous , met niveau code upload je débute!

Sinon une "faille" marrante est lorsque tu dépasses max_post_size ... tu te retrouves avec $_POST vide, alors que l'utilisateur a bien rempli son formulaire et a envoyé ses fichiers.

Très drôle aussi, on peut imaginer du coup

if ($_POST)
{
// traitement des données
}

Ainsi, l'utilisateur se retrouve avec un formulaire vierge, alors qu'il vient d'envoyer les données, et il comprend plus rien. Et toi non plus ^^

Je vous laisse chercher les solutions à ce problème ^^

D'ailleurs par la même occasion tu pourrais stocker la taille de ton image dans la bdd pour avoir un meilleur contrôle de l'affichage

.

Tu peux m'expliquer ? Ça m’intéresse. Comment ça avoir un meilleur contrôle ? Ça optimise ? Ça fait quoi ?

Si par exemple tu veux faire afficher des images les unes en dessous des autres dans un div et que les hauteurs ne sont pas renseignées, ton div va faire l'accordéon au début de son affichage, le temps que toutes les images soient chargées pour trouver la hauteur réelle. En effet la page s'affiche avant que toutes les images soient chargées. Alors que si les hauteurs des images sont renseignées le navigateur s'en servira pour calculer la hauteur du div et donc il aura, dès le chargement de la page, la bonne dimension.

merci à tous , met niveau code upload je débute!

je t'ai donné les solutions... dans les liens de mon premier message :
Soit tu veux quelque chose de rapidement efficace et modulaire et tu utilise la classe.
Soit tu as plus de temps et pour mieux comprendre tu regarde le tuto.

Si par exemple tu veux faire afficher des images les unes en dessous des autres dans un div et que les hauteurs ne sont pas renseignées, ton div va faire l'accordéon au début de son affichage, le temps que toutes les images soient chargées pour trouver la hauteur réelle. En effet la page s'affiche avant que toutes les images soient chargées. Alors que si les hauteurs des images sont renseignées le navigateur s'en servira pour calculer la hauteur du div et donc il aura, dès le chargement de la page, la bonne dimension.

Ok d'accord, merci AB, j'aime bien apprendre des choses sur l'optimisation donc c'est toujours ça de pris.