Message de tobozo de phpsecure.info, que je remercie infiniment au passage. :-) :
- Effectivement, merci beaucoup à tobozo pour cette petite étude ! Merci de lui faire passer ce message à l'occasion... :) -- ProgFou
salut Thierry
Je viens vers vous pour vous demander s'il serait possible de sécuriser un moteur wiki que j'utilise :
http://www.wikini.net
Le moteur est effectivement troué d'un peu partout, une sécurisation s'impose ... vois plutot le résultat de ce test (qui révèle une faille xss) :
http://www.wikini.net/wakka.php?wiki=%3Cscript%3Ealert(document.cookie);%3C/script%3E
Pour ce qui est de sécuriser, c'est effectivement possible, mais
pas par nos soins, ou alors sans aucune exhaustivité (comme pour
cet audit qui se limite a l'examen d'un seul fichier du projet).
Le code est bien indenté mais on peut y faire l'inventaire des erreurs de design suivantes :
- mauvaise gestion des magic_quotes
- mauvais filtrage des données utilisateur
- messages d'erreur trop bavards (certains révelent le principe de
- stockage des documents 'page/fichier' + '.php')
La zone de code sur laquelle les développeurs vont devoir se focaliser
pour pallier le probleme de xss qui se situe dans wakka.php
$wiki = preg_replace("/^\//", "", $wiki);
// split into page/method
if (preg_match("#^(.+?)/([[:alnum:]_]*)$#", $wiki, $matches)) list(,
$page, $method) = $matches;
else if (preg_match("#^(.*)$#", $wiki, $matches)) list(, $page) = $matches;
suggestion :
Il faut renforcer les expressions régulieres, celles qui sont en
place actuellement sont trop peu suffisantes ...
Pour toute autre question similaire je te suggere de poster dans
le forum de phpsecure, les réponses y seront aussi précises que
les questions ...
@+
tbz
Cette faille est en effet ennuyeuse du fait qu'elle peut être exploitée à partir d'un autre site que wikini.net.
Elle ne devrait cependant pas être très compliquée à corriger. Le code suivant permet de la corriger, mais il faudrait voir s'il n'a pas trop d'impact sur le reste :
// Split into page/method
// (Note : it test if the wiki name doesn't contain some characters that should permit an XSS)
if (preg_match("#^([A-Za-z0-9]+?)/([A-Za-z0-9_]*)$#", $wiki, $matches)) list(, $page, $method) = $matches;
else if (preg_match("#^([A-Za-z0-9]*)$#", $wiki, $matches)) list(, $page) = $matches;
else
{
echo "Interdit !";
exit;
}
--
CharlesNepote
Je pense que tu as oublié le "_" dans la partie des regexp qui concerne la page. De plus je pense que tu dois pouvoir faire ca en une seule regexp :
(à tester)
if (preg_match('#^(\w+)(/(\w+))?$#',$wiki,$matches)) list(,$page,,$method)=$matches;
else{
}
Rappel: \w = [A-Za-z0-9_]
On doit même pouvoir ajouter les caratères accentués mais il y a déjà un débat à se sujet. --
GarfieldFr
Oui pour l'expression unique si testée.
Attention, il me semble que \w est sensible à la locale, raison pour laquelle nous avons choisi d'utiliser les classique [A-Za-z0-9] (avant de passer aux variables).
Pourquoi le _ ? Je ne me souviens plus que nous pouvions l'utiliser...
--
CharlesNepote
Personnellement, je préfère mon code qui a l'avantage d'être bien clair et robuste. On raffinera par la suite si vous le voulez bien... en revanche je veux bien que quelqu'un :
- soit fasse la mise à jour de la branche CVS 0.4
- soit documente un peu comment faire pour que je puisse le faire, je ne suis pas très à l'aise sur ces choses
--
CharlesNepote
Pour ma part j'ai mis ceci :
$search = array ("'<script[?>]*?>.*?</script>'si", // Supprime le javascript
"'<[\/\!]*?[^<?>]*?>'si", // Supprime les balises HTML
"'([\r\n])[\s]+'", // Supprime les espaces
"'&(quot|#34);'i", // Supprime les entites HTML
"'&(amp|#38);'i",
"'&(lt|#60);'i",
"'&(gt|#62);'i",
"'&(nbsp|#160);'i",
"'&(iexcl|#161);'i",
"'&(cent|#162);'i",
"'&(pound|#163);'i",
"'&(copy|#169);'i",
"'&#(\d+);'e"); // Evaluation comme PHP
$wiki = preg_replace ($search, "", $wiki);
Vos avis ... ?
ThierryBazzanella
Mon avis c'est que :
- tu procèdes par une exclusion négative : pas ça, ni ça, etc. ; il est donc possible que tu ais oublié des choses
- WikiNi 0.5 ne contiendra a priori plus de [A-Za-z0-9] mais seulement des constantes, plus faciles à manier ; ton code ne vas pas dans ce sens : cf. le début du fichier wakka.php dans le CVS : http://cvs.gna.org/viewcvs/wikini/wikini/wakka.php?rev=1.54&content-type=text/vnd.viewcvs-markup
- tu exclues ¢ alors que nous allons peut-être l'utiliser dans la 0.5 pour gérer les caractères accentués ; cet exemple de ¢ montre bien pourquoi il vaut mieux passer par une désignation positive de ce qui est accepté (et qui sera bientôt l'objet de constantes modifiables)
Cela dit, il y avait du bon et tu ne connaissais certainement pas encore ces objectifs de la 0.5.
--
CharlesNepote
Ok, Charles, juste un bout de code récuperer sur Google :p. Que proposes tu en attendant la 0.5 ? --
ThierryBazzanella
- Je vais m'occuper tout de suite de porter la modification de CharlesNepote sur la branche 0.4. -- ProgFou
Ou doit t'on se rendre pour télécharger le path de sécurité de la 0.4.1rc ? --
ThierryBazzanella
- Le suffixe rc veut dire Release Candidate, c'est à dire un candidat pour la version 0.4.1. Or la version 0.4.1 est déjà sortie ! ;-) Ce devrait donc plutôt être soit une 0.4.1.1, 0.4.1a ou 0.4.2 (a priori notre choix ici). Je suis en train de vérifier encore une fois et de tester le patch de CharlesNepote et le miens. Ensuite je l'annoncerai sur WikiNi dans les pages WikiNiChangeLog041 et WikiNiChangeLog050 et ici même. Pour finir, je laisserai CharlesNepote changer le numéro de version et publier une archive .tar.gz en ligne sur la page DownloadWikiNi. -- ProgFou
Avant d'envoyer la purée, toutes les entrées de données ont été vérifiées dans tous les fichiers ???
Parce que, vous êtes rapide, et ca fait un sacré boulot ! .... bref ..... des wagons de courage à vous ... --
ThierryBazzanella
Une collaboration inter communautaire serait trés enrichissante pour les parties.
A lire donc mon invitation vers
http://www.phpsecure.info/v2/board/list.php?f=6
Un souhait commun : Un
WikiNi sécurisé !!!
A bientot ... --
ThierryBazzanella
Excellente idée... j'espère seulement que cela ne va pas nous donner trop de boulot ;) --
CharlesNepote
Il y a un moyen simple de sécuriser Wikini : c'est de supprimer le lien "Editer cette page". --
DavidDelon
- Hihihi... je sais qu'on a déjà discuté de tout ça et je suis d'accord avec toi. Cela dit, lorsque à un problème donné, on peut trouver une solution simple et sans impact fonctionnel, pourquoi s'interdire de la mettre en oeuvre ? -- CharlesNepote
WikiNi à l'heure actuelle dans sa version 0.4.1 est fonctionnel et non sécurisé. Et nous, utilisateurs de
WikiNi aimerions avoir un système
sécurisé. Un code sécurisé est un code ou toutes les entrées ont été correctement formatées.
Mais bon, je ne vous apprends rien :p Peut-être une 0.4.2 en vue ?
Bon courage à tous, et merci infiniment pour tout le travail déjà effectué ... --
ThierryBazzanella
Vous l'avez sans doute compris, par cette remarque je voulais pointer du doigt que le risque, à vouloir tout vérouiller , c'est qu'on ne dispose plus d'un outil utilisable, je rappelle qu'une des différences de Wikini (et de son papa Wakka) par rapport aux grands moteurs wiki en php et sa facilité d'installation et d'utilisation (bien qu' il y ait encore beaucoup de progrès à faire).
J'avoue également que le ton du message ci-dessus m'a hérissé (il n'etait peut être pas destiné à être rendu public) :
quand je lis
messages d'erreur trop bavards (certains révelent le principe de stockage des documents 'page/fichier' + '.php'), je ne comprends pas : Wikini est un logiciel libre non ? N'importe qui à accès au source et peut se rendre compte de la façon dont sont stockés les actions ?
Je reconnais effectivement que le ton du message est hérissant, mais comme donner son avis technique n'est pas une affaire de diplomatie je suis curieux de savoir quel ton s'impose en de telles circonstances a part un ton parano a tendance psychotique ;-)
L'explication c'est qu'en php il y a une regle de sécurité basique qui dit qu'il faut idéalement mettre display_errors=0. Si cette regle n'est pas respectée alors je la signale.
Pourquoi cette regle existe est une évidence : si le code d'un projet libre est dispo pour les développeurs de facon évidente, il l'est bien moins pour les script kiddies qui eux utilisent plus l'instinct que la pédagogie pour exploiter des trous qu'ils ne comprennent pas forcément dans leur profondeur. --tobozo
100% ok à ce niveau, tobozo psicotte un peu mais bon, cela n'enlève rien au problème posé :p --
ThierryBazzanella
Pour ce qui est du filtrage des données utilisateur je ne vois pas de problème à la solution proposé par Charles. --
DavidDelon
- En fait si, il y a un souci... CharlesNepote et moi-même (ProgFou) en avons discuté un peu tout à l'heure (via Jabber) et il s'avère qu'un manque de précision dans la spécification des noms des pages dans WikiNi nous empêche de corriger correctement ce bogue sans risquer de créer des problèmes pour certains utilisateurs... Je m'explique : dans WikiNi il n'est pas spécifié explicitement la définition technique d'un nom de page, excepté par le fait que les liens vers ces pages devraient (mais pas obligatoirement) correspondre à des MotWiki si l'on souhaite qu'ils soient construits automatiquement ; pourtant WikiNi n'empêche absolument pas de créer une page ayant un nom qui n'est pas un MotWiki (sans-doute est-ce là le fond du problème) et permet même de forcer des liens vers ces pages avec la syntaxe des doubles crochets [[ ]]. N'ayant jamais eu de contrainte sur le nom des pages jusqu'ici, je suspecte que plusieurs site utilisant WikiNi aient des noms de pages avec des caractères divers et variés ne faisant pas partie de la définition d'un MotWiki, comme par exemple le tiret '-', le souligné '_' ou encore des accents ou autres symbols de ponctuation... Par conséquent, il faut ouvrir un débat sur la définition d'un nom de page dans WikiNi : quels sont les caractères acceptés pour un nom de page ? Pour ma part je propose de n'accepter que les lettres, les chiffres, le souligné et éventuellement les accents (sachant qu'on ne devrait les implémenter officiellement dans la 0.5.0). La correction complète de cette faille XSS ne pourra se faire qu'au terme d'une décision sur cette définition. -- ProgFou [Ok -- DavidDelon ] [Parfait -- ThierryBazzanella]
Perso, j'ai opté pour la possibilité de mettre quasi-tout dans le nom de mes pages (sauf < et quelques petites autres choses)... J'ai aussi opté pour laisser aux gens la possibilité de mettre n'importe quelle balise HTML dans les pages ou les commentaires (d'ailleurs, j'ai incorporé un WYSIWYG à
WikiNi, y'a plus de syntaxe Wiki !). Du coup, pour la sécurité, j'ai opté pour un truc très simple, mais je ne sais pas si ça suffit :
- '<!' remplacé en '<!'
- '<?' remplacé en '<?'
- '<#' remplacé en '<#'
- 'script' remplacé en 'scr|pt'
- 'data' remplacé en 'da|a'
Plus de détails sur l'ensemble des modifs que j'ai faites :
http://wiki.vi5.org
--
GuilainOmont
Et au passage, merci pour
WikiNi, ça déchire :-)