TWC LE PREMIER SITE EN CMS DES TOUTOOS

Commentaire de yoman64 le 13/01/2008 12:08:39

Salut,
J'ai regardé brievement le code et quelques petits détails me sautes aux yeux.

1. Dans le fichier /inc/login.php
Tu devrais remplacer ton  if ( empty($pseudo) ) { suivis du else if ( empty($motdepasse) ) {
Par un simple  if ( empty($pseudo) ||  empty($motdepasse)) {  puisque de toute manière les deux conditions afficherais le meme contenu.

2. Dans ce genre de code la POO améliorerait beaucoup la clarté du code, surtout quant à la base de données, une classe d'abstraction de base de données serait bien. (évidement c'est subjectif, ça ne regarde que moi)

3. TU NE SÉCURISE PAS TES REQUETES. ça m'a frappé, tu sécurises pas les données que tu passes dans tes requetes mysql, l'injection mysql est très dangereuse, je te suggère de t'informer sur mysql_real_escape_string. C'est une erreur très grave.

4. Dans la page d'inscription :

#  $resultmembre = @mysql_query("SELECT * FROM `membres` WHERE `id`='".$id."'");
# if(@mysql_num_rows($resultmembre) > 0) {
#
# while ( $membres = @mysql_fetch_assoc($resultmembre) )
# {
# $id2 = $membre['id'];
# echo "Ce pseudo éxiste déja dans notre base de donnée.<br><br><a href='?page=suscribe'>Recommencer son inscription</a> ou <a href='index.php'>Revenir a l'accueil</a>";
# }

Ton while ne sert a rien, je me trompe ?.

J'ai essayer de pas être trop dur, je ne veux que t'aider hein ;). Ton projet est interessant, mais je vais attendre la prochaine version pour tester,
Bonne continuation

Commentaire de lmame le 14/01/2008 09:42:05

Salut :)
Même remarque pour la sécurité, en Mysql il suffit de penser juste au typage et à mysql_real_escape_string (enfin, en gros...), autant le faire dès le départ que de se fader tous ses fichiers (quand on a pas de POO, c'est là qu'on voit que ça sert :) ).

Autre chose, toujours au niveau de la sécurité, apparemment le mot de passe est en "clair" dans la base de données. Déjà tu pourrais le stocker en md5() là aussi en une fonction tu sécurises un peu plus.

Sinon le fait de mettre des "@" devant toutes les commandes mysql juste pour cacher la misère en cas de plantage me paraît pas trèèèès judicieux :)
Autant faire une classe pour ça et traiter l'erreur éventuel (surtout si comme tu le dis c'est un proto et que tu veux savoir où et pourquoi ça plante).

Pour stocker les infos bdd tu crées un fichier php (system/data.php) mais sans vérifier s'il a bien été crée (ou si tu as les droits pour dans le dossier en question) ou pas et ensuite tu l'utilises dans pas mal de fichiers. Là encore pas de tests à l'ouverture en création du fichier.

Autre chose, dans system/modules.php (et aussi ailleurs):
if(empty($name) OR empty($actif))
if ( $actif == 'Oui' AND $name == 'Navigation')
tu devrais voir les opérateurs de comparaison booléens, || pour le OR et && pour le AND.

Commentaire de neigedhiver le 17/01/2008 23:39:08

Mais pourquoi tant de sites limitent le nombre de caractères pour le mot de passe ? C'est la nouvelle notion de sécurité ?
Tant que c'est pas uniquement des caractères alphanumériques... (un peu comme ici quoi...)

Commentaire de yoman64 le 18/01/2008 11:37:05

8 c'est pas beaucoup personnelement mes mots de passe font souvent plus de 10 charactères...

Je vois pas l'interêt de limiter, si l'utilisateur l'oubli alors il n'a qu'à le redéfinir... On ne devrait pas forcer l'utilisateur à avoir un mot de passe de moin de 8 charactères.

En plus tes mots de passe sont stockés en clair dans la base, très mauvaise idée... et tu ne sécurise toujours pas tes requêtes avec mysql_real_escape_string.

C'est bien beau dire que ce n'est qu'un prototype et que tu feras mieu plus tard, mais rien ne t'empêche de tenir compte des remarques que l'on te fait ici. Toutes les remarques ci-dessus sont excellentes et tu devrais les écouter. Lorsque tu poste un script tu dois être prêt a l'améliorer au besoin, sinon il est totalement inutile ;) .

Commentaire de satan2006 le 21/01/2008 11:20:56

si vous avez fait un tour sur le site de TWC vous remarquerai que la V2 est en cours de construction et que la correction des erreures de sécurité sont entrain d'être traiter! Je travaille dur sur le déroulement de l'histoire de ce CMS!

Deplus il ne sert a rien de devenir agressif! car pour information mais je ne parle d'ésormais plus de prototype. Ensuite pour les mot de passe je ne sait pas encore quoi faire au niveau du cryptage! Soit j'en défini un a l'inscription que l'utilisateur pourra toujour modifier ou alors je ne limite pas le nombre de caractère!

Commentaire de neigedhiver le 21/01/2008 12:28:05

Salut,

Pour compléter ce que dit Lmame :

Concernant la longueur du mot de passe, il est totalement inutile de la limiter. Cela n'apporte rien à personne, si ce n'est une plus grande facilité pour un hacker qui utilise un brute force. Plus le mot de passe est long, plus les caractères sont spéciaux, plus c'est difficile à trouver.
Un mot de passe uniquement numérique se casse "assez facilement". Si on rajoute des lettres, majuscules/minuscules, on augmente la difficulté. Si on rajoute des caractères non alphanumériques (!%$£=- etc) on augmente encore la difficulté. Et plus le mot de passe est long, plus on continue d'augmenter la difficulté.
Voilà pour ce qui est de l'intérêt de la non limitation du nombre de caractères (ou alors, à 30, mais pas 8, c'est vraiment trop peu).

Pour ce qui est de la méthode : tu hésites entre générer un mot de passe aléatoire modifiable plus tard, et ne pas limiter le nombre de caractères... Ca n'a rien à voir.
Commence par ne pas limiter la longueur du mot de passe, ou alors à une grande longueur (30 par exemple ; et encore, on peut tout à fait choisir une phrase comme "mot de passe", d'où le mot anglais passphrase).
Ensuite, soit l'utilisateur choisit son mot de passe à l'inscription, soit le mot de passe est généré aléatoirement et envoyé par mail.
Pourquoi ne pas envisager les deux possibilités et laisser l'administrateur la méthode qu'il préfère ?
Voire même une troisième possibilité : l'admin choisit le mot de passe en inscrivant manuellement les membres (pour un site privé par exemple).
Il faut de toute façon que le mot de passe soit modifiable ensuite par l'utilisateur, puisque c'est la loi qui le demande (cf site de la CNIL).

Enfin, il existe plusieurs algorithmes pour 'hasher' un mot de passe. md5 le fait sur 32 octets, sha1 sur 40, et d'autres encore sur davantage. A toi de choisir le plus performant sans sacrifier les performances.

Commentaire de yoman64 le 21/01/2008 12:37:22

Salut neigedhivers,

En fait après avoir lu un article en ce qui concerne le stockage de mot de passe par hash, le plus performant c'est jamais ce qu'on veut, au contraire :).
Plus l'algo est lent, plus le hash est long a calculer, plus quelqu'un va avoir de la difficulter à le bruteforcer ensuite puisqu'il doit calculer le hash de chaque possibilité. Et puis le calcul du hash sur un CMS c'est négligeable, c'est pas comme si il allait avoir 2000 connexions/inscriptions par minute.

J'ai peut être mal compris ce que tu voulais dire par contre, répond moi si je me trompe!

Commentaire de lmame le 21/01/2008 13:26:15

Oui pour le md5() c'était juste pour donner une méthode basique de cheminement et de code pour php. Comme ce genre de chose se retrouve très aisément dans les codes / tutos sur internet ça permets déjà de lui donner une idée.

Après bien évidemment ce n'est pas la panacée le md5, loin de là pour les raisons déjà données :)

Ensuite pour la sécurisation totale d'un soft ça peut aller beaucoup plus loin, du log systématique des query / login / accès à l'utilisation de logs générés par mod_security que l'on peut installer pour Apache. Mais bon comme vous l'avez dit, tout est une question de juste milieu tout de même :)

Commentaire de yoman64 le 21/01/2008 17:15:13

Niveau débutant, et hash le mot de passe n'est pas récupérable justement, c'est ça le but...