Discussion :

[Mopige]

Bonjour,

Sur mon site, le visiteur a la possibilité de calculer la somme de risques à l'aide d'une application disponible en ligne. Les données sont saisies dans un tableau à l'aide d'un formulaire qui les transmet dans une application .PHP qui en fait la somme par simulation de Monte-Carlo. Lorsque le nombre de risques est supérieur à 6, ce qui est fréquent, cette saisie manuelle peut être laborieuse.
J'envisage donc de donner au visiteur la possibilité de charger un fichier Excel dans lequel il aura au préalable décrit la liste des risques à traiter. Cela simplifie énormément l'utilisation de cette application en ligne.

Cependant je m'interroge sur la sécurité de cette opération. La lecture du fichier sur le serveur est écrite en PHP.
1/ Charger un fichier Excel (en fait .csv) rempli par le visiteur peut-il être dangereux si celui-ci est mal intentionné. Ce risque est-il réel ?
2/ Cette pratique de charger un fichier est-elle utilisée par d'autres sites ?
3/ Comment se protéger pour réduire ou supprimer le risque ?
4/ L'hébergeur peut-il disposer d'un moyen de protection (filtre, etc.) pour sécuriser ce genre de pratiques. ?

Merci de bien vouloir m'informer sur ces questions.

[mathieu]

du moment que la donnée vient de l'extérieur du script, un utilisateur malveillant peut toujours essayer de mettre des données imprévues.
donc même avec l'upload d'un fichier, vous devez faire les mêmes vérifications que vous feriez avec une saisie d'un texte dans un formulaire.

[Mopige]

Merci pour votre réponse.

Dans le formulaire je vérifie que les données de chaque risque, donc de chaque enregistrement, correspondent à la nature et au format attendus. Ces données sont transmises au PHP après vérification.

Dans le cas où les données sont saisies via un fichier csv, cette vérification a lieu dans l'application PHP, après la lecture de ces données. La différence c'est que s'il y a un problème, il apparait sur le serveur et non sur le PC client.
Je suppose que c'est donc plus risqué de lire un fichier que de saisir un formulaire.

Qu'entendez vous par données imprévues. Chaque enregistrement contient 5 données: un string de 3 caractères maximum et 4 numériques. Si je vérifie dans le PHP les formats et la cohérence des données avant utilisation, ça devrait être clean ? Que puis-je faire d'autre ?

Merci

[binarygirl]

En réalité, le danger se situe moins au niveau du format du fichier, que de sa prise en charge par le mécanisme d'upload.
Il existe des techniques permettant de contourner la vérification de l'extension, et aussi de la destination du fichier.
Ce que vous ne voulez pas, c'est qu'un attaquant puisse uploader un webshell .php dans un répertoire de votre site web. Quelques exemple de techniques: https://portswigger.net/web-security/file-upload

Quand votre routine commence à parser le fichier, celui-ci est déjà présent sur le serveur, normalement dans le répertoire temp ou dans un répertoire dédié. Il faut donc veiller à ce que le fichier ne puisse pas atterrir ailleurs et avec un nom arbitraire.

[mathieu]

pour prendre un exemple de données imprévues, on pourrait partir d'un formulaire qui affiche 3 jours et l'utilisateur indique pour chaque jour s'il est présent ou en congé en sachant qu'il a droit qu'a un seul jour de congé.
donc l'envoi du formulaire va par exemple envoyé "jour 1 présent, jour 2 congé, jour 3 présent". et même si en javascript, il y a une vérification de la cohérence pour qu'un seul jour soit férié, un utilisateur malintentionné pourrait envoyer "jour 1 congé, jour 2 congé, jour 3 congé".
c'est pour cela que les données doivent être revérifiées coté serveur en php.

et que ces données soit écrites dans une balise "input" ou envoyé dans un fichier csv, cela ne pose pas de souci particulier et demande la même vérification des données dans les 2 cas.

dans votre cas si j'ai bien compris, le résultat est un simple affichage de calculs et il n'y a rien qui est enregistré coté serveur ?

[Mopige]

Bonjour,

Je reprends un peu tard la discussion de la semaine dernière en suivant les éléments importants de vos réponses.

1/ Prise en charge du fichier

Le danger se situe moins au niveau du format du fichier, que de sa prise en charge par le mécanisme d'upload. Il existe des techniques permettant de contourner la vérification de l'extension, et aussi de la destination du fichier.
Ce que vous ne voulez pas, c'est qu'un attaquant puisse uploader un webshell .php dans un répertoire de votre site web. Quelques exemple de techniques: https://portswigger.net/web-security/file-upload
Quand votre routine commence à parser le fichier, celui-ci est déjà présent sur le serveur, normalement dans le répertoire temp ou dans un répertoire dédié. Il faut donc veiller à ce que le fichier ne puisse pas atterrir ailleurs et avec un nom arbitraire.

J'ai mis en place les contrôles suivants : type MIME autorisé, extension autorisée, taille inférieure ou égale au nécessaire.

2/ Les données imprévues
Chaque enregistrement contient 5 données: un string de 3 caractères maximum et 4 numériques. A la lecture de chaque enregistrement dans le PHP, je vérifie les formats et la cohérence des données avant utilisation. Par exemple, selon la valeur du string, les numériques doivent respecter certaines règles.

3/ Enfin le site est sécurisé avec des plugins adaptés chez l'hébergeur.

Le risque n'est jamais nul, il ne peut être que réduit. Si vous voyez d'autres mesures, n'hésitez pas à m'en faire part.

Merci

[binarygirl]

J'ai mis en place les contrôles suivants : type MIME autorisé, extension autorisée, taille inférieure ou égale au nécessaire.

Oui c'est la base, encore que le type MIME est envoyé par le browser, il peut être absent ou spoofé donc c'est sur l'extension qu'il faut se focaliser. Mais après il faut voir comment vous enregistrez le fichier côté serveur. Si vous acceptez le nom d'origine, vous risquez d'avoir des surprises, par exemple que se passerait-il si j'envoie un upload d'un nommé fichier ../../doc.png ? Sera-il bien sauvegardé là où vous pensez qu'il devrait être ? Et le contrôle de l'extension peut être contourné si votre méthode de vérification n'est pas blindée. Par exemple, un regex qui ne prendrait pas en compte le multiline. Il serait donc intéressant de voir le bout de code qui traite l'upload côté serveur pour voir s'il y a des failles exploitables que vous n'avez pas envisagé.

Aucune idée des plugins dont il est question, donc je ne peux pas commenter là-dessus. J'imagine qu'il y a probablement un WAF mais ce n'est pas une protection absolue pour autant: il existe des méthodes de contournement et d'obfuscation, pas forcément évidentes mais automatisables quand même.

[Mopige]

Bonjour et merci pour votre réponse

Ci-dessous une copie du code avant l'ouverture du fichier

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
else {//	Lecture d'un fichier;
	//Le nom original du fichier sur le disque du visiteur
	$fich_lecTablo = $_FILES['avatar']['name'];	
	echo "Nom du fichier : ",$fich_lecTablo,"<br/>";

	//$taille = filesize($_FILES['avatar']['tmp_name']) La taille du fichier en octets
	$taille = $_FILES['avatar']['size'];	//.
	echo "Taille en octets : ",$taille,"<br/>";

	//Le type du fichier. Par exemple, cela peut être « image/png ».
	$tipe = $_FILES['avatar']['type'];	
	echo "Type : ",$tipe,"<br/>";

	//L'adresse vers le fichier uploadé dans le répertoire temporaire.
	$adress = $_FILES['avatar']['tmp_name']; 
	echo "Adresse : ",$adress,"<br/>";

	//Le code d'erreur, qui permet de savoir si le fichier a bien été uploadé
	$coderr = $_FILES['avatar']['error'];
	echo "Code erreur : ",$coderr,"<br/>";
	
	//Début des vérifications de sécurité...
	$extensions_valides = '.csv';
	$extension_upload = strrchr($_FILES['avatar']['name'], '.');	
	
	if($tipe != "application/vnd.ms-excel") {
		echo "Vous devez charger un fichier de MIME conforme";
		exit;}
		
//	echo "Extension : ",$extension_upload,"<br/>";
	if($extension_upload != $extensions_valides) {
		echo "Vous devez uploader un fichier de type csv";
		exit;}
		
	$taille_maxi = 800;
	if($taille > $taille_maxi)	{ 
		echo 'Le fichier est trop gros.';
		exit;	}

Trois exemples de résultats

Nom du fichier : tablo3.csv
Taille en octets : 170
Type : application/vnd.ms-excel
Adresse : C:\wamp64\tmp\php39AF.tmp
Code erreur : 0
Puis affichage des enregistrements. C'est OK
Risque 1 0.5 uni 0 0 5
Risque 2 0.5 uni 0 0 5
Etc.

Nom du fichier : un essai.txt
Taille en octets : 879
Type : text/plain
Adresse : C:\wamp64\tmp\php16F7.tmp
Code erreur : 0
Vous devez charger un fichier de MIME conforme exit

Nom du fichier : tablo1.csv
Taille en octets : 1000
Type : application/vnd.ms-excel
Adresse : C:\wamp64\tmp\phpA6C8.tmp
Code erreur : 0
Le fichier est trop gros. exit

Le fichier est envoyé par le formulaire suivant:

Code :

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

1
2
3
4
5
6
<form method="POST" action="simuRisq2.php" enctype="multipart/form-data" target="_blank">
	Entrer le nom du fichier : <input type="file" name="avatar"> 
	<!-- Le bouton affiche "Parcourir"   name : nom du champ d'entrée correspondant à type -->
	<input type="hidden" name="MAX_FILE_SIZE" value="5000"><br>
	<input type="submit" name="envoyer" value="Envoyer le fichier" class="bouton" width: 300px><br>
</form>

L'utilisateur doit sélectionner le fichier dans son explorateur de fichiers puis cliquer sur envoyer. Cela permet-il d'envoyer un upload d'un fichier nommé../../doc.png ?

Sera-t-il bien sauvegardé là où vous pensez qu'il devrait être ?

L'adresse est affichée avant l'ouverture du fichier. C:\wamp64\tmp\phpA6C8.tmp en local.
Est-il utile de s'assurer que l'adresse est valide avec l'hébergeur ?

Le site est développé avec WordPress est contient les plugins suivants : Akismet Anti-Spam: Spam Protection / BBQ Firewall / WPS Hide Login. De plus l'hébergeur annonce plusieurs mesures de sécurité.

Je suppose que j'ai balayé en (grande ?) partie le champ des vulnérabilités.

Merci

[binarygirl]

L'utilisateur doit sélectionner le fichier dans son explorateur de fichiers puis cliquer sur envoyer. Cela permet-il d'envoyer un upload d'un fichier nommé../../doc.png ?

Un attaquant ne va pas utiliser le navigateur mais plutôt utiliser un robot ou un outil qui permet de passer outre les limitations imposées par le navigateur.
Notez que même avec un navigateur on peut faire des choses, on peut éditer une requête pour la renvoyer avec des modifications, changer les headers etc.
Et puis il y a les outils généralistes comme Curl qui offrent déjà beaucoup de possibilités, sans même parler des outils dédiés aux tests d'intrusion. Ici un exemple dédié à un cas de figure bien précis (upload d'un fichier zip). SQLmap permet aussi d'uploader un webshell en tirant parti d'une injection SQL, si certaines conditions sont réunies.

Dans le code que vous avez posté, je ne vois pas de vulnérabilité immédiate, le contrôle de l'extension semble OK je crois. Par contre, la question que je me posais concernait le reste du traitement. Une fois que le fichier a passé tous les contrôles, comment le renommez-vous et le déplacez-vous ? Si vous vous basez sur le nom d'origine sans le "sanitiser" il peut y avoir un risque. C'est donc la suite du code qui est intéressante et sensible

Dans les plugins que vous avez cité, BBQ semble être un WAF et peut protéger contre certaines attaques. Par exemple: "Directory traversal attacks" mais là on parle sans doute de la manipulation d'URL, je doute que ça va examiner les uploads de fichiers mais sait-on jamais. Je n'ai pas regardé en détail le fonctionnement de ce module. Il faut garder présent à l'esprit qu'un WAF rend l'exploitation plus difficile mais pas forcément impossible, et ne vous protégera pas toujours de vos propres erreurs de programmation.

[Mopige]

Bonjour, et merci pour vos explications.

Une fois que le fichier a passé tous les contrôles, comment le renommez-vous et le déplacez-vous ? Si vous vous basez sur le nom d'origine sans le "sanitiser" il peut y avoir un risque. C'est donc la suite du code qui est intéressante et sensible

Code :

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

1
2
3
4
5
6
	$fich = fopen($fich_lecTablo, "r");	
	if ($fich) {   // si le fichier a bien été ouvert
		while (false !== $ligne = fgetcsv($fich)) {  
				$lignetab = explode(";", $ligne[0]);
				$R[$i][1] = (float)$lignetab[0];
				Etc.  lecture des enregistrements pour les ranger dans une variable

Est-ce correct ?
Merci
(Je parts en vacances pendant une semaine)

[binarygirl]

J'ai probablement mal compris le but du programme, apparemment vous lisez le fichier en boucle pour en extraire les données mais il est abandonné à la fin du processus. Il restera donc dans le répertoire temp, jusqu'au reboot ou jusqu'à ce que vous le supprimiez. Dans ce cas de figure, mes remarques ne s'appliquent pas vraiment donc. Ceci dit, je pense que votre code pourrait facilement planter si on injecte un fichier avec des données invalides, par exemple si une ligne de contient pas de ; ou que le premier élément n'est pas convertible en float. Mais ça n'en fait pas pour autant une vulnérabilité, à moins d'identifier un buffer overflow dans une des fonctions invoquées, et de calibrer la charge juste comme il faut, ce qui est complexe.

De toute façon, vous devriez tester votre propre code en lui donnant des saletés, par exemple des fichiers binaires ou exécutables par exemple, avec une extension .csv et voir comment il réagit. Il faut quand même blinder le parsing du fichier car une page qui plante ça fait mauvaise impression pour celui qui l'utilise.

[Mopige]

Bonjour,

Merci pour votre réponse.
De retour de vacances (avec le COVID pour me tenir compagnie), je reprends le fil de ce long échange.

1/ OUI le fichier .csv est abandonné à la fin de la lecture. Je suppose qu'il disparaitra automatiquement à la fin du traitement PHP.

2/ OUI le code plantera si les données ne passent pas les contrôles. Les règles à respecter sont clairement affichées.

3/

Tester le code en lui donnant des saletés.

Dans l'ordre, les contrôles avant la lecture sont : le MIME / l'extension / la taille
J'ai essayé un fichier "stylegs.css" en remplaçant css par csv.
Il a passé les 2 premiers contrôles mais a été bloqué car il était trop gros. J'ai alors augmenté la taille maxi admise, et il a passé le 3ème contrôle. Par contre il s'est planté lors de la lecture avec un Warning à l'instruction suivante :

Code :

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

1
2
3
4
5
6
if (true == $fich = fopen($fich_lecTablo, "r")) {   // si le fichier a bien été ouvert     ( ligne 94)
		lecture …
	}
	else{
		echo 'Fichier non lisible';
		exit;}

Warning: fopen(stylegs.csv): Failed to open stream: No such file or directory in C:\wamp64\www\somRisq\simuRisq2.php on line 94 (if (true == $fich = fopen($fich_lecTablo, "r"))…

Le traitement est interrompu avec le message Fichier non lisible.

Q1. Si le MIME est défini en fonction de l'extension, je ne vois pas l'intérêt de conserver le 2 tests ?
Q2. Est-il possible de ne pas afficher le Warning lors de l'exécution car il n'est pas utile d'indiquer l'adresse du code au visiteur.

Merci

[binarygirl]

Dans l'ordre, les contrôles avant la lecture sont : le MIME / l'extension / la taille
J'ai essayé un fichier "stylegs.css" en remplaçant css par csv.

Notez que ça reste un fichier texte, j'essaierais aussi avec un fichier binaire, par exemple un .exe pour voir comment le script réagit...

Il a passé les 2 premiers contrôles mais a été bloqué car il était trop gros. J'ai alors augmenté la taille maxi admise, et il a passé le 3ème contrôle. Par contre il s'est planté lors de la lecture avec un Warning à l'instruction suivante :
...
Warning: fopen(stylegs.csv): Failed to open stream: No such file or directory in C:\wamp64\www\somRisq\simuRisq2.php on line 94 (if (true == $fich = fopen($fich_lecTablo, "r"))…

Il y a aurait un peu de gestion d'erreur à ajouter, il y a lieu de vérifier que l'upload a effectivement réussi, sinon pas la peine d'aller plus loin. Jetez un coup d'oeil à la doc, par exemple la superglobale $_FILES contient un attribut 'error'.

Q1. Si le MIME est défini en fonction de l'extension, je ne vois pas l'intérêt de conserver le 2 tests ?
Q2. Est-il possible de ne pas afficher le Warning lors de l'exécution car il n'est pas utile d'indiquer l'adresse du code au visiteur.

Le type MIME est envoyé par le navigateur donc vous ne pouvez lui faire confiance. C'est justement une technique d'attaque, car je peux envoyer une requête avec un fichier malicieux et un header MIME type qui ne correspond pas. Ce qui important à vérifier dans le cas présent c'est l'extension du fichier.

Pour des tâches de ce genre je pense que cela aurait du sens d'utiliser une classe qui a été éprouvée depuis longtemps, par exemple: https://github.com/verot/class.upload.php

Avantages:

• ça évite de réinventer la roue, la valeur ajoutée est quand même limitée
• ça évite des erreurs de débutants, voire d'induire des risques de sécurité
• une partie de la gestion d'erreur est déjà prise en charge

C'est particulièrement pertinent dès lors que la fonctionnalité (ici le file upload) peut créer un trou de sécurité si elle mal codée.
Évidemment il est encouragé de l'étudier pour voir comment ça fonctionne. Même si le code mentionné plus haut fait plus que ce qui est nécessaire, ce serait peut-être une bonne idée de l'utiliser et le ré-utiliser pour vos projets.

Je ne suis pas si vous codez dans un framework particulier, mais ils sont généralement fourgués avec des fonctionnalités de ce genre prêtes à l'emploi pour que justement vous n'ayez pas à réinventer la roue, mais puissiez vous concentrer sur le reste.