Core de FluxBB : téléchargement des avatars dans profile.php.

Wan · 29-03-2012 12:01:55

Bonjour.

Je suis actuellement en train de travailler sur une mod et j'avais une question.
Dans profile.php, à la section "avatars", il y a ce paramètre passé en hidden via le formulaire de téléchargement :

<input type="hidden" name="MAX_FILE_SIZE" value="<?php echo $pun_config['o_avatars_size'] ?>" />

Ensuite il y a une vérification de la taille du fichier :

// Make sure the file isn't too big
if ($uploaded_file['size'] > $pun_config['o_avatars_size'])
				message($lang_profile['Too large'].' '.forum_number_format($pun_config['o_avatars_size']).' '.$lang_profile['bytes'].'.');

N'y a-t-il pas la une redondance de sécurité ? Peut-on, sans craindre pour la sécurité, supprimer la vérification ?

PascL · 29-03-2012 18:44:11

Euh c'est pas le seul endroit où tu trouveras une redondance.

En PHP, tu dois mettre des sécurités sur les formulaires (pour que l'utilisateur ne mette pas n'importe quoi), et tu dois mettre des sécurités sur le traitement de ces formulaires (pour vérifier que les données sont valides et/ou conformes à ce que tu attends).

Parce que quelqu'un de mal intentionné pourra toujours contourner la sécurité du formulaire: le formulaire est visible côté client et éditable sans trop d'efforts (c'est seulement du html).

Exemple simple:
Je copie la page d'upload d'avatar sur mon disque dur, je modifie quelques données dedans et je renvoie vers la page de ton site... Si tu supprimes la vérification lors du traitement, je peux mettre ce que je veux comme taille de fichier...

Wan · 29-03-2012 18:49:16

Autre question sur le fonctionnement de profile.php, il y a quelque chose que j'ai du mal à comprendre.
En effet, après de multiples tests, en local avec wampserver :
si j'essaie d'envoyer une image dont la taille dépasse $pun_config['o_avatars_size'] (déclaré dans Administration/options), le message d'erreur renvoyé est :

Fichier trop volumineux. Le serveur refuse son envoi.

Ce message d'erreur correspond à $lang_profile['Too large ini'] qu'on trouve ici (et seulement ici) :

// Make sure the upload went smooth
			if (isset($uploaded_file['error']))
			{
				switch ($uploaded_file['error'])
				{
					case 1: // UPLOAD_ERR_INI_SIZE
					case 2: // UPLOAD_ERR_FORM_SIZE
						message($lang_profile['Too large ini']);
						break;

Cela voudrait donc dire c'est une erreur renvoyée directement par PHP et que mon fichier dépasserait upload_max_filesize défini dans php.ini. Or ce n'est pas le cas ! (upload_max_filesize est réglé à 4Mo chez moi).
Normalement je devrais avoir ce message d'erreur là :

La taille du fichier dépasse le maximum autorisé 10 240 octets.

Qui correspond à cette partie :

// Make sure the file isn't too big
if ($uploaded_file['size'] > $pun_config['o_avatars_size'])
	message($lang_profile['Too large'].' '.forum_number_format($pun_config['o_avatars_size']).' '.$lang_profile['bytes'].'.');

D'ailleurs, si je change la valeur de upload_max_filesize, c'est toujours le premier message d'erreur qui apparaît si la taille de mon fichier est supérieur à cette valeur.

Conclusion, y a quelque chose qui cloche ou que je n'arrive pas à saisir...

adaur · 29-03-2012 19:16:37

Salut Wan,

C'est normal! (voir doc)

UPLOAD_ERR_FORM_SIZE

    Valeur : 2. Le fichier téléchargé excède la taille de MAX_FILE_SIZE, qui a été spécifiée dans le formulaire HTML.

C'est à cela que sert le input hidden cité plus haut. C'est une protection supplémentaire en quelque sorte.

Wan · 29-03-2012 22:30:13

adaur a écrit :

Salut Wan,
C'est normal! (voir doc)
UPLOAD_ERR_FORM_SIZE

    Valeur : 2. Le fichier téléchargé excède la taille de MAX_FILE_SIZE, qui a été spécifiée dans le formulaire HTML.
C'est à cela que sert le input hidden cité plus haut. C'est une protection supplémentaire en quelque sorte.

Merci Adaur ! Je viens de m'apercevoir de ce fait.
Du coup on a peu de chance de rencontrer l'erreur :

La taille du fichier dépasse le maximum autorisé 10 240 octets.

C'est dommage, car cela renseigne plus l'utilisateur que :

Fichier trop volumineux. Le serveur refuse son envoi.

Car en fait le serveur ne refuse rien du tout, c'est vraiment profile.php qui le refuse (d'où ma confusion)....

Wan · 29-03-2012 22:36:29

PascL a écrit :

Euh c'est pas le seul endroit où tu trouveras une redondance.
En PHP, tu dois mettre des sécurités sur les formulaires (pour que l'utilisateur ne mette pas n'importe quoi), et tu dois mettre des sécurités sur le traitement de ces formulaires (pour vérifier que les données sont valides et/ou conformes à ce que tu attends).
Parce que quelqu'un de mal intentionné pourra toujours contourner la sécurité du formulaire: le formulaire est visible côté client et éditable sans trop d'efforts (c'est seulement du html).
Exemple simple:
Je copie la page d'upload d'avatar sur mon disque dur, je modifie quelques données dedans et je renvoie vers la page de ton site... Si tu supprimes la vérification lors du traitement, je peux mettre ce que je veux comme taille de fichier...

Merci pour ces explications PascL ! C'est vrai que je n'ai pas (encore) l'esprit assez tordu pour devenir un bon hacker, donc je ne pense pas à tous les "trucs" imaginables.
Cependant, je prépare une mod qui se veut la plus sécurisée possible, donc ma question n'était pas innocente... wink

WinuX · 29-03-2012 22:37:42

Une protection html (même js), ça s'appelle pas une protection tongue
Donc j'affirme : input totalement useless.

Pour modifier la taille max des avatars, ça se joue du coté de l'administration, dans les options.

Wan · 29-03-2012 23:24:42

WinuX a écrit :

Pour modifier la taille max des avatars, ça se joue du coté de l'administration, dans les options.

Oui, ça je sais !

WinuX a écrit :

Une protection html (même js), ça s'appelle pas une protection
Donc j'affirme : input totalement useless.

Je suis d'accord, je faisais simplement remarquer que l'information fournie était erronée en cas de dépassement de la taille max des avatars.

En fait, je cherche à simplifier le téléchargement d'avatars pour les utilisateurs, il faut donc beaucoup de protections... qui seront gérées par PHP bien sûr ! wink

WinuX · 30-03-2012 09:40:47

Si tu veux simplifier pour les utilisateurs, ajoutes un remote upload (ou la mod "avatar par url" mais faudra corriger quelques pseudo failles).
C'est très pratique wink

Wan · 30-03-2012 11:02:12

WinuX a écrit :

Si tu veux simplifier pour les utilisateurs, ajoutes un remote upload (ou la mod "avatar par url" mais faudra corriger quelques pseudo failles).
C'est très pratique

Je suis parti de la mod "Avatar par url" que j'avais déjà adaptée pour FluxBB 1.4 (voir ICI), mais il y a des grosses failles de sécurité que je suis en train de corriger. J'y rajoute un redimensionnement automatique. J'ai presque fini maintenant que j'ai eu vos réponses à mes interrogations.
Merci à vous ! wink

adaur · 30-03-2012 16:09:50

@Wan: Je ne suis personnellement pas fan de l'avatar distant, je trouve que c'est potentiellement ouvrir son forum à des failles pour rien quand un avatar ne prend que quelques Ko à être stocké.

Par rapport au script que tu as posté sur le .org, le seul truc que je rajouterai est une vérification de l'URL pour vérifier qu'elle ne contient pas de "php" dans son titre. En effet, une image X.php.png peut être interprétée par le serveur comme un fichier PHP.

Wan · 30-03-2012 22:13:16

adaur a écrit :

@Wan: Je ne suis personnellement pas fan de l'avatar distant, je trouve que c'est potentiellement ouvrir son forum à des failles pour rien quand un avatar ne prend que quelques Ko à être stocké.
Par rapport au script que tu as posté sur le .org, le seul truc que je rajouterai est une vérification de l'URL pour vérifier qu'elle ne contient pas de "php" dans son titre. En effet, une image X.php.png peut être interprétée par le serveur comme un fichier PHP.

J'essaye de pourvoir à ces failles...
Pour la taille du fichier lié à l'URL et pour éviter un éventuel "overflow", j'ai mis en place une lecture de celui-ci en streaming avec un arrêt au-delà d'une limite fixée par l'administrateur.
J'ajoute que dans le cas d'un URL envoyé, ce n'est pas vraiment un "avatar distant" dans le sens où, dans la mod que je suis en train de finaliser, le fichier image lié est réellement enregistré sur le serveur dans /img/avatar/. Il ne sera donc jamais affiché dans le viewtopic de façon distante.
Pour l'extension "déguisée", j'ai dealé avec ".gif", ".jpg", ".png" pensant que ".gif.php" par exemple était potentiellement dangereux. Je ne savais pas que ".php.gif", par exemple, était potentiellement dangereux aussi.
Est-ce le cas ?
Si ça l'est je ferai un preg_match supplémentaire...
Merci de vos conseils...

Dernière modification par Wan (30-03-2012 22:39:20)

WinuX · 31-03-2012 05:25:05

Qu'on me corrige si j'me trompe, mais si tu fais un remote upload comme ça à l'air d'être le cas, alors inutile de te soucier de php. Il est exécuté coté serveur, donc tu ne télécharges aucuns scripts php, juste l'image.

Wan · 31-03-2012 08:37:15

C'est un peu le cœur du problème et la raison de cette discussion...
Il "paraît" qu'un script peut être inclus dans un fichier image (avec le gif en particulier).
Effectivement, je ne pense pas qu'il ne se passe quoi que ce soit pendant l'upload : celui-ci est fait en streaming, par paquets d'octets, donc aucune exécution de script possible.
Ensuite je crée un objet image avec la chaîne composée des octets uploadés en utilisant imagecreatefromstring. Là déjà on peut avoir un doute sur une exécution possible ou non (ça me paraît peu probable).
Puis, l'objet image est transféré en tant que fichier dans /img/avatars. Là non plus, pas d'exécution possible.
Par contre, c'est lors de l'affichage de l'image dans viewtopic qu'il pourrait se passer quelque chose si un script est caché, non ?
Je veux bien avoir votre avis sur ce fait.

[Edit] Si on rajoute un redimensionnement de l'objet image, je ne vois pas comment un script "survivrait" à cette opération...

Dernière modification par Wan (31-03-2012 12:24:47)

adaur · 31-03-2012 16:43:38

Wan a écrit :

Est-ce le cas ?

Oui, sur certains serveurs des fichiers PHP .php.png (ou autre) peuvent être exécutés. Exemple: http://adaur.monespace.net/image.php.png

<?php
echo 'ceci est du code PHP';
?>

Même si d'autres vérifications sont faites, un petit preg_match ne ferait pas de mal happy .

Dernière modification par adaur (31-03-2012 16:47:39)

Wan · 31-03-2012 17:25:01

adaur a écrit :

Wan a écrit :
Est-ce le cas ?
Oui, sur certains serveurs des fichiers PHP .php.png (ou autre) peuvent être exécutés. Exemple: http://adaur.monespace.net/image.php.png
<?php
echo 'ceci est du code PHP';
?>
Même si d'autres vérifications sont faites, un petit preg_match ne ferait pas de mal .

Belle démonstration ! plus_un
Je m'en occupe donc...

Wan · 31-03-2012 18:26:20

Bon après mure réflexion et beaucoup de tests j'ai décidé de n'autoriser le remote upload qui si la bibliothèque gd est activée (sinon, seul l'upload classique est activé).
Cela résout les problèmes de sécurité :

L'upload se fait par paquets pour pouvoir gérer la limite de taille maximale du fichier image d'origine.
Puisque l'upload donne une chaîne d'octets, et que je passe par un imagecreatefromstring, je n'ai pas besoin de m'occuper du type d'image téléchargée : en particulier, un fichier comme Adaur vient de montrer ne sera pas reconnu en tant qu'image et ne sera donc pas accepté.
Pour éviter quand même tout problème, et puisque la fonction redimensionnement est prévue, je ferai systématiquement passer l'image "à la moulinette" :
- si l'image est trop grande à l'origine, la réduction détruira tout script éventuellement contenu dans la chaîne d'octets.
- si l'image est d'origine de taille acceptable, je la "gonflerai" à 125 % puis la réduirai à 80% pour retrouver la taille d'origine : il y aura une très légère perte de qualité (pas grave pour un avatar) mais cette opération double détruira aussi tout script éventuellement contenu.

Le temps de m'occuper de l'install_mod et des fichier lang et la mod sera bientôt dispo. Merci pour vos conseils !

WinuX · 31-03-2012 18:33:36

Le preg_match se contourne malheureusement hmm , un simple url_rewriting et le .php n'est plus happy

Il manque juste un header pour que ce soit considéré comme une image adaur:

header('Content-type: image/png');

Sans ça, impossible d'up l'image happy

Reste plus qu'à tester quand tu uploades l'image depuis l'url distance si le code est toujours interprété par php happy

Wan · 31-03-2012 19:02:02

Quand j'aurai finalisé la mod, je vous laisserai tester : malheureusement, je n'ai aucun fichier image contenant un script sous la main pour essayer... wink
Mais ça m'étonnerait qu'il passe à travers ma "moulinette"... happy

adaur · 31-03-2012 19:36:41

Wan a écrit :

- si l'image est d'origine de taille acceptable, je la "gonflerai" à 125 % puis la réduirai à 80% pour retrouver la taille d'origine : il y aura une très légère perte de qualité (pas grave pour un avatar) mais cette opération double détruira aussi tout script éventuellement contenu.

Ce n'est pas la peine de faire ceci: en plus de prendre du CPU pour rien, c'est inutile car un simple renommage complet de l'image (comme FluxBB le fait) rend déjà inoffensif un script.

Pour tester ta mod, cherche un shell c99 en PHP wink

Wan · 31-03-2012 22:07:26

adaur a écrit :

Ce n'est pas la peine de faire ceci: en plus de prendre du CPU pour rien, c'est inutile car un simple renommage complet de l'image (comme FluxBB le fait) rend déjà inoffensif un script.

Bon, ça me rassure (d'autant qu'après essais, la qualité de l'image était douteuse, ce qui prouve qu'un imagecopyresampled n'est pas une opération très performante, qualitativement parlant (en plus de "prendre du CPU" comme tu le dis).
Et puis je pense qu'avec la méthode employée (mise en mémoire d'une chaîne d'octets réinterprétée comme image), tous les traitements se font sur la chaine d'octets en question (l'objet image) sans jamais qu'il n'y ait d'interprétation de code. Pour l'enregistrement de l'image finale, je suis resté sur la méthode proposée par FluxBB avec renommage complet, donc ça devrait aller.

adaur a écrit :

Pour tester ta mod, cherche un shell c99 en PHP

O.k., je vais essayer ! wink

Wan · 01-04-2012 19:36:53

Une petite remarque à propos des avatars : dans leur administration, il est possible de régler leur largeur, hauteur ou taille à des valeurs négatives... hmm déjà que zéro c'est plus que limite... siffle
Je sais que ces valeurs sont enregistrées dans la table config et que celle-ci ne contient que des varchar(255). Du coup on peut mettre à peu près n'importe quoi comme valeur : sur les avatars, si on entre autre chose que des entiers, soit ce sont des décimaux arrondis à des entiers, soit cela renvoie zéro ; mais les entiers négatifs sont acceptés ce qui n'est pas normal.

Dernière modification par Wan (01-04-2012 19:37:13)

Otomatic · 02-04-2012 10:25:13

Wan a écrit :

... mais les entiers négatifs sont acceptés ce qui n'est pas normal.

Bonjour,

Les rapports de bug pour FluxBB, c'est là

FluxBB.fr

#1 29-03-2012 12:01:55

Core de FluxBB : téléchargement des avatars dans profile.php.

#2 29-03-2012 18:44:11

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#3 29-03-2012 18:49:16

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#4 29-03-2012 19:16:37

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#5 29-03-2012 22:30:13

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#6 29-03-2012 22:36:29

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#7 29-03-2012 22:37:42

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#8 29-03-2012 23:24:42

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#9 30-03-2012 09:40:47

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#10 30-03-2012 11:02:12

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#11 30-03-2012 16:09:50

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#12 30-03-2012 22:13:16

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#13 31-03-2012 05:25:05

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#14 31-03-2012 08:37:15

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#15 31-03-2012 16:43:38

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#16 31-03-2012 17:25:01

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#17 31-03-2012 18:26:20

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#18 31-03-2012 18:33:36

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#19 31-03-2012 19:02:02

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#20 31-03-2012 19:36:41

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#21 31-03-2012 22:07:26

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#22 01-04-2012 19:36:53

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

#23 02-04-2012 10:25:13

Re : Core de FluxBB : téléchargement des avatars dans profile.php.

Pied de page des forums