[php] Des risques pour l'upload de jpeg, gif, png ou psd ???

(reprise du message précédent)

Graph-Site a écrit :
"Refuser les fichiers à double extension potentielle, en foo.xxx.yyy, me semble une nécessité évidente pour un script d'upload."
Oui, je crois que la méthode la plus simple serait de refuser les points, autre que celui de l'extension. par contre, il va falloir que je cherche comment faire ça...

Regex ou explode...

Graph-Site a écrit :
en tout cas merci à tous d'avoir participer à ce sujet

De rien Merci aussi pour toutes les pistes qui ont été lancée ici. Je vais pouvoir mettre un système d'upload au point.

a+

on se retouve d'ici quelques jours sur ce post pour comparer nos scripts et trouver d'éventuelles failles, ça te dis ??? (je ne préfère pas utiliser un de mes scripts avant que quelqu'un de plus compétent l'ai regardé un peu...)

@ +

<edit>
Pour ce qui est d'interdir un autre point que celui de l'extension :

$nbr_points=substr_count($nom_image, ".");
if ($nbr_points>'1'){
//on annule l'uplaod et on sort notre joli mesage :
echo 'Le nom de votre image ne doit pas comporter de point (.), exepté celui de l\'extension, merci.'; }
else{
//c'est bon
}

Théoriquement ça marche ??? </edit>
Modifié par Graph-Site (05 Aug 2005 - 21:35)

Graph-Site a écrit :
on se retouve d'ici quelques jours sur ce post pour comparer nos scripts et trouver d'éventuelles failles, ça te dis ???

Oui, dans quelques jours

Graph-Site a écrit :
<edit>
Pour ce qui est d'interdir un autre point que celui de l'extension :

$nbr_points=substr_count($nom_image, ".");
if ($nbr_points>'1'){
//on annule l'uplaod et on sort notre joli mesage :
echo 'Le nom de votre image ne doit pas comporter de point (.), exepté celui de l\'extension, merci.'; }
else{
//c'est bon
}

Théoriquement ça marche ??? </edit>

Il me semble que ca devrait marcher, mais je n'ai pas tester.


A dans quelques jours alors

Ok
Je n'ai pas testé non, plus, donc comme c'est moi qui est codé ce petit truc, il est fort probable que ça sorte une erreur de synthaxe (je suis tête en l'air souvent )

@ +

Comme le préconise, les expressions régulières sont bien utiles.

C'est multi-plateforme, multi-langages et même dans un shell ...
N'accepter que [A-Za-z0-9] _ et - par exemple. Il faut encore vérifier qu'une fois uploadé, c'est bien un fichier (et donc pas une commande, un répertoire, ...). Pas de double // qui ont aussi un effet détourné, etc etc

Par exemple :
If (ereg("[A-Za-z0-9_]|\\-",$nom_image)){
// c'est bon
}
else{
// pas bon
}

j'ai un peu de mal c'est sans doute faux, mais bon, ça ne fait que 10minutes que je suis me documente sur ces expressions régulières...

<edit>Cela ne fonctionnait pas... j'ai donc lu 3 ou 3 trucs supplémentaires concernant les exp. reg. Voila ce que ça donne :

<?php
$nom_image="testfaf-_";

if(ereg("^[[:alnum:]_\-]+$", $nom_image))
{
echo 'Le nom de l\'image est correct.';
} else {
echo 'Le nom de l\'image est incorrect. Merci d\'utiliser uniquement des caractères alphanumériques, ainsi que le tiret (-) et l\'underscore (_)';
} 
?>

J'ai testé, ça semble marcher. si ça peut vous servir, n'hésitez pas...
</edit>


@+
Modifié par Graph-Site (06 Aug 2005 - 12:51)

Salut,
Voici le script que j'ai obtenu, avec une ou 2 heures de bricolage (oui, je sais le codage est loin d'être beau...

<?php
if (isset($_GET['action']) AND $_GET['action'] == "validation")
{
// Si le formulaire est validé, on définit les dimensions autorisées.
$largeur_max = 4000;
$hauteur_max = 3000;
$poids_max = 5242880; //5mo = 5120ko = 5242880 octets

$erreur = "";
$poids_max_txt = round($poids_max / 1024)." ko";

if (is_uploaded_file($_FILES['image']['tmp_name']))
{
if ($infos_img = getimagesize($_FILES['image']['tmp_name']))
{
if ($infos_img[0] > $largeur_max)
{$erreur .= "L'image a une largeur de ".$infos_img[0]."px alors que la largeur maximale est $largeur_max px.<br />";}
if ($infos_img[1] > $hauteur_max)
{$erreur .= "L'image a une hauteur de ".$infos_img[1]."px alors que la hauteur maximale est $hauteur_max px.<br />";}

if ($_FILES['image']['size']>$poids_max)
{
$erreur .= "L'image est trop lourde. Son poids ne doit pas dépasser ".$poids_max_txt.".<br />";}
if ($infos_img[2] >= 6 OR $infos_img[2]==4) // 1=GIF, 2=JPG, 3=PNG, 4=SWF, 5=PSD, 6=BMP, 7&8=TIFF, 9=JPC, 10=JP2, 11=JPX, 12=JB2, 13=SWC, 14=IFF
{$erreur .= "L'image doit être au format .gif, .jpg, .psd ou .png. Merci<br />";}
if (file_exists("/images/".$image_name))
{$erreur .= "Il y a déjà une image avec ce nom.<br />";}
if ($erreur == "")
{
move_uploaded_file($image,"images/".$image_name);
$erreur = "no";            
}
}
else{
$erreur .= "Ce n'est pas une image qui a &eacute;t&eacute; upload&eacute;e.<br />";
}
}
else{
$erreur .= "Aucune image n'a &eacute;t&eacute; upload&eacute;e.<br />";
}

if ($erreur=='no'){
echo '<div style="background-color:#FFFFFF; border:2px dashed #000000; margin:auto; margin-top:100px; height:150px; width:500px; text-align:center;">';
echo 'L\'upload s\'est correctement déroulée.<br />';
echo '<a href="upload.php">Cliquez-ici pour uploader une nouvelle image.</a>';
echo '</div>';
}
if ($erreur!='no') {
echo '<div style="border:1px dashed #0000FF; margin:auto; margin-top:50px; width:500px; text-align:center;">';
echo $erreur.'<br />';
echo '<a href="upload.php">Cliquez-ici pour retourner au formulaire d\'upload</a>';
echo '</div>';
}

}
else{
//Si le formulaire n'a pas été validé.
?>
<div style="padding:20px; background-color:#FFFFFF; border:2px dashed #000000; margin:auto; margin-top:100px; height:150px; width:500px; text-align:center;">

<form name="upload" enctype="multipart/form-data" method="POST" action="upload.php?action=validation">
<input type="hidden" name="MAX_FILE_SIZE" value="7542880" />
<!-- Upload autorisée 7mo via formulaire, puis message d'erreur si poids>5mo --> 
<input type="file" size="35" name="image" /><br>
<input type="submit" value="Uploader" />
</form>
</div>
 <?php } ?>

Donc en principe, on ne peut uploader rien d'autre que des images (gis, png, jpeg, psd) puisque si le getimagesize() échoue l'uplaod échoue également. Donc en principe c'est sécurisé.
Surtout que dans le script finale, les images seront enregistrés sous cette forme : "nombre_aleatoire.ext".
Pouvez vous faire des petits esais pour en vérifier la sécurité SVP ?


merci !

@ +

Bonjour,

Le miens n'est pas encore fini, juste une petite remarque, le getimagesize ne sécurise pas tout... On peut très bien retourné une image avec un fichier.php, donc, tu sauras faire ton getimagesize dessus... Mais si tu vérifie les doubles extensions, tu ne devrais pas avoir de problème...

Je poste ma fonction tantôt ou demain.

a+

agilis a écrit :
On peut très bien retourné une image avec un fichier.php

Encore faut-il que celui-ci passe par un parser PHP, ce qui n'est pas le cas des fichiers uploadés.

Salut,

Dis moi si je me trompe, ce qui est fort possible :
Pour retourner une image avec un fichier php, il faut que celui-ci soit exécuté. Or là, il ne peut pas être exécuté avant que la procédure d'upload ne soit terminée, donc il n'y a pas de risques si ?
Et comme de toute façon comme je l'ai dis, les fichiers seront renommés pendant l'upload, seul l'extension d'origine sera gardée.

ça pause toujours un problème ou pas à votre avis ?

@ +

djfeat a écrit :

Encore faut-il que celui-ci passe par un parser PHP, ce qui n'est pas le cas des fichiers uploadés.

Pourquoi il ne serait pas parser? Si tu arrive à exécuter du php avec un fichier de ce type : monscript.php.jpg, quand tu fais un getimagesize, il me sembe que le fichier est exécuter, donc parser. Mais si on vérifie qu'il n'y a qu'une extesion, le problème est réglé.

@Graph-Site > AMHA, ça ne devrait plus poser de problèmes. Toutes les vérifications nécessaires sont effectuée. Juste une chose, je ne ferais pas un nombre aléatoire pour renommer la photo... Il se peut que deux photos et le même nom alors... C'est rare mais ca m'est déjà arriver. Donc, la dernière ajoutée écrase l'autre. Moi, j'utiliserais soit uniqid() mais c'est un peu long, soit un fichier texte qui contient un numéro que l'on incrément à chaque upload...


a+

Salut,
Le fichier est exécuté lors du getsizeimage ?
Zut, dans ce cas, mon changement de nom ne vas pas suffir puisqu'il s'effectue après...
Tu aurais un exemple de script qui retourne une image STP ? et qui pourrait selon toi passer mon script d'upload, pour que je puisse tester.
Si tu n'as pas le temps, ce n'est pas grave, j'en ferai plusieurs, il y en aura bien un qui correspondra à celui auquel tu penses ^^

Pour le nombre aléatoire, il aurait été à 15 chiffres, mais tu as raison, je ne vais pas faire comme ça...je me renseigne merci.

@ +

agilis a écrit :
Pourquoi il ne serait pas parser? Si tu arrive à exécuter du php avec un fichier de ce type : monscript.php.jpg, quand tu fais un getimagesize, il me sembe que le fichier est exécuter, donc parser.

Une image ça ne s'execute pas, getimagesize ne va pas envoyer ton fichier local vers un parser.

J'ai fait des tests, j'arrive à faire un getimagesize sur un fichier php :
http://www.phpgo.org/test.php
Le code de test.php est :

<?php var_dump(getimagesize('http://www.phpgo.org/design/menu.php')); ?>

Le fichier php testé est :
http://www.phpgo.org/design/menu.php
Et le code :

<?php
header('Content-type: image/png');
if(isset($_GET['text'])) $text = $_GET['text']; else $text = '.';
if(isset($_GET['s'])) $s = $_GET['s']; else $s = 14;
$font = '../menu.ttf';
$size = imagettfbbox($s,0,$font,$text);
$dx = abs($size[2]-$size[0]);
$dy = abs($size[5]-$size[3]);
$xpad = 15;
$ypad = 15;
$im = imagecreate($dx+$xpad,$dy+$ypad);
$fond = imagecolorallocate($im,255,255,255);
$black = imagecolorallocate($im,102,102,102);
imagettftext($im, $s , 0, (int)($xpad /2), $dy +(int)($ypad/2), $black, $font, $text);
imagecolortransparent($im,$fond);
imagepng($im);
imagedestroy($im);
?>

Il me semble donc que getimagesize exécute le fichier s'il doit l'exécuter... Il faudrait testé sur un mutualisé d'OVH (je trouve pas le mod_ort dans mon httpd.conf)

a+

Edit : je m'était complètement trompé...
Modifié par agilis (07 Aug 2005 - 10:15)

De toute façon un fichier uploadé a un nom temporaire ($_FILES['nom_du_champ']['tmp_name']) sans double extension ni caractère bizarre donc la question du .php.jpg ne se pose même pas.

agilis a écrit :
J'ai fait des tests, j'arrive à faire un getimagesize sur un fichier php :
http://www.phpgo.org/test.php
Le code de test.php est :

<?php var_dump(getimagesize('http://www.phpgo.org/design/menu.php')); ?>

Oui mais là ça n'a rien à voir avec un fichier local (encore moins un fichier uploadé), dans ton exemple menu.php est executé sur phpgo.org et getimagesize ne traite que l'image résultante.

djfeat a écrit :
Oui mais là ça n'a rien à voir avec un fichier local (encore moins un fichier uploadé), dans ton exemple menu.php est executé sur phpgo.org et getimagesize ne traite que l'image résultante.

Exactement, j'avais pas pensé à ca. Donc, aucuns risques.

Je n'ai pas trop compris ce que vous dîtes (hehe, on se sent c*n des fois !)
Mais dans le cas du script que tu as testé, cela ne pose de toute façon aucun problème, car si je ne me trompe pas, puisque selon ton header c'est une image...le script ne peut pas être nocif.
Mais bon, on n'est pas obligé de créer une image de cette façon.

Donc, si je comprends bien mon script actuel est sécurisé, en principe ?

@ +

Graph-Site a écrit :
Je n'ai pas trop compris ce que vous dîtes (hehe, on se sent c*n des fois !)
Mais dans le cas du script que tu as testé, cela ne pose de toute façon aucun problème, car si je ne me trompe pas, puisque selon ton header c'est une image...le script ne peut pas être nocif.
Mais bon, on n'est pas obligé de créer une image de cette façon.

Donc, si je comprends bien mon script actuel est sécurisé, en principe ?

@ +

En principe oui.
Mon script peut très bien être nocif, je peux faire ce que je veux avant de renvoyer l'image... par exemple, récupérer le contenu d'un fichier php et me l'envoyer par mail...

a+

ah, ok, je me trompais sur le fonctionnement du header alors...

merci pour ton aide en tous cas !

@ +