Faille critique dans Les Visiteurs 3.0 Beta
Par Matt le lundi 18 février 2008, 23:53 - PHP - Lien permanent
Fin 2006, j'avais commencé à retravailler une vieille application de mesure d'audience en PHP : Les Visiteurs, de J-Pierre DEZELUS (phpInfo.net).
Je l'utilisais depuis 2000 et, comme elle n'était plus maintenue par son auteur, j'y avais apporté au fil du temps pas mal de petites améliorations, que j'ai finalement décidé de rendre publiques, sous la forme d'une version 3, toujours au stade de beta car je n'ai pas beaucoup de temps à y consacrer.
Toutefois, une mise à jour s'impose aujourd'hui, à cause d'une faille de sécurité majeure, qui est en partie responsable du défacage d'Infobidouille le 14/02/2008.
Le problème a une origine toute bête : ma fainéantise... En effet, Les Visiteurs a été développée à une époque où les problèmes de sécurité n'étaient pas encore aussi critiques qu'aujourd'hui, et où register_globals était à on par défaut dans PHP. Tous les développeurs PHP connaissent la dangerosité de ce paramètre, et c'est pourquoi il est de nos jours très fortement recommandé de le mettre à off, comme c'est le cas par défaut dans les versions actuelles de PHP.
Mais comme Les Visiteurs avait été développé à l'ancienne
, l'application ne fonctionnait pas avec register_globals à off. Pour palier rapidement au problème sans revoir tout le code existant, je me suis contenté de mettre un extract($_GET) au début du fichier config.inc.php. Erreur fatale, cette petite ligne rend le code aussi peu sécurisé que lorsque register_globals est à on : en passant un lvc_include_dir dans l'URL, on peut utiliser certains scripts de l'application pour exécuter du code malicieux et compromettre la sécurité du serveur en escaladant petit à petit les droits (en profitant de certains failles de PHP et du noyau Linux, un accès root est possible...).
Je n'ai pas vraiment le temps de publier une nouvelle version en ce moment, mais voici ce qu'il faut modifier dans include/config.inc.php pour corriger la faille :
extract($_GET);
à remplacer par :
extract($_GET);
if (isset($_GET["lvc_include_dir"])) {
exit;
}
Ainsi, l'exécution de tous les scripts de l'application (ils font tous appel au fichier de config) sera interrompue si un attaquant tente de forcer la valeur de lvc_include_dir via l'URL. Ce correctif peut aussi s'appliquer à la version 2, mais il faudra alors l'appliquer à tous les fichiers utilisant la variable $lvc_include_dir : admin/admin.php3, include/config.inc.php3, include/library.inc.php, include/menus.inc.php, include/new-visitor.inc.php3, include/update-agents.php, include/update-isp.php, image-vis.php3 et index.php.
Il s'agit bien entendu d'un correctif temporaire, en attendant que Les Visiteurs 3.0 soit finalisée, avec une réécriture du code pour pouvoir se passer du extract et remplacer la variable $lvc_include_dir par une constante, pour empêcher toute injection d'une nouvelle valeur.
Je profite également de ce billet pour vous suggérer très fortement de bloquer directement les include sur des fichiers distants, d'autant plus que ce n'est pas extrêmement compliqué à faire :
- sous PHP 5.2 et supérieurs, il suffit de positionner le paramètre allow_url_include à off dans php.ini,
- dans les versions antérieures de PHP, il faut positionner allow_url_fopen à off dans php.ini (attention, contrairement au nouveau paramètre allow_url_include, allow_url_fopen s'applique à toute opération sur un fichier distant, même une simple lecture).
Vous pouvez aussi empêcher ces opérations avec l'extension Suhosin, qui bloquera également bon nombre d'autres vecteurs d'attaques.
Commentaires
Je souhaite moi aussi mettre à jour "Les Visiteurs".
Est-ce que tu es avancé dans ce travail ?
Je rêve d'un "Les Visiteurs" plus sécurisé et avec des graphiques plus sympa ou personalisables.
Non, je n'ai pas avancé. J'ai peu de temps pour m'en occuper en ce moment, et j'ai d'autres priorités (je déménage dans moins d'un mois...).
Mais si tu es motivé pour faire évoluer Les Visiteurs, nous pourrions peut-être travailler ensemble ?
J'ai un serveur CVS de disponible, et j'ai un tas d'idée d'améliorations qui ne demandent qu'à être implémentées :-)
Moi aussi, c'est assez dur de bosser à 200% dessus.
Mais si on est 2 à bosser, ce serait plus simple.
On pourrait s'occuper du projet en tant que leaders, mais fonctionner en communauté pour améliorer le script.
Par contre, pour ma part, je ne souhaite pas que le script devienne plus complexe. Il y a phpMyVisites pour cela. Trop complexe à mon goût d'ailleurs, et vraiment trop lourd en base de données sur des gros sites.
Les Visiteurs, c'est un script utile pour "renifler" l'efficacité d'un référencement/positionnement tout en ayant une vue globale des pages intéressantes et du profil des visiteurs.
Bon alors tu vas pas être content, parce que j'ai déjà rajouté des fonctionnalité supplémentaires ^^ (taux de visites significatives, stats sur les FAI...).
Mais l'archi de base du script est relativement modulaires, donc y a moyen de pas utiliser ces fonctions pour ceux qui n'en veulent pas (on pourrait même proposer deux packages, l'un reprenant juste les fonctions de base, l'autre contenant toutes les nouvelles fonctions).
Concernant l'idée d'une communauté, pourquoi pas, mais faudra voir comment administrer le CVS dans ce cas (bon, ça doit pas être super compliqué, mais pour l'instant mon CVS tourne en single user, il me sert que à synchroniser mon workspace entre mon PC et mon Mac).
Si t'es toujours motivé dans trois semaines (après mon déménagement, et si ma connexion Internet est rapidement rétablie...), on pourra en reparler à ce moment.