NPDS : Gestion de contenu et de communauté

Voila, j'ai publier sa tout à l'heure sur securityfocus donc je vous prévient quand même ^^

EDIT DE Boris :
J'ai déplacé ca dans un message caché. Ainsi, seul la TNT peut le voir, pour que perzonne n'en profite pour éventuellement faire des dégats sur des sites existants. ;-)

Message édité par : Boris / 05-05-2007 06:41

On va regarder ca.

Merci, Gu1ll4um3r0m41n

Cependant, il existe un système de filtrage automatique des variables, donc je doute que cela soit très grave, mais on regarde quand même, on n'est jamais trop prudent. :-)

Message édité par : Boris / 05-05-2007 06:44

j'ai fait des test avant de publier...
Je doute que votre "systeme de filtrage" decode le texte encodé en base_64, le filtre (addslash & co) et le reencode ?...

Message édité par : Gu1ll4um3r0m41n / 05-05-2007 15:39

:=! Merci Gu1ll4um3r0m41n
;-) Boris, un bon chmod est très utile pour parer à cela ? (ou fichier caché et lecture seule sous Win)
A +

Non, cette faille est au niveau du code PHP et non de la gestion des fichiers

Et je crois effectivement qu'elle est efficace

Si elle ne l'étais pas, j'aurais pas poster ce topic ni averti des sites de sécurité ni codé un exploit -_-

C'est tout à fait exact Gu1ll4um3r0m41n et nous t'en remercions.

Cependant, dans les corrections de sécurité que tu préconises concernant la création des cookies, il me semble qu'il y à un tout petit problème.
"
$cookie[0] = inval($cookie[0);
"

Aurais-tu oublié quelque chose ?

File mainfile.php (npds v5.10), add after line 665:
"
$cookie[0] = intval($cookie[0]);
"
Encore merci.

Message édité par : neo_machine / 08-05-2007 16:33

ch'tite correction:
---> $cookie[0] = inval($cookie[0);<= intval($cookie[0]);<-----
Fonction intval() qui permet de convertir une variable en numérique entier. rien à voir avec la sécurité mais elle prémuni contre les injections sql.
Pour les utilisateurs de versions antérieures à Sable, rechercher ça:


function cookiedecode($user) {
   $stop=false;
   if (!empty($_GET)) {
      if ($_GET[user]!="") { $stop=true; $user="BAD-GET";}
   } else if (!empty($HTTP_GET_VARS)) {
      if ($HTTP_GET_VAR[user]!="") { $stop=true; $user="BAD-GET";}
   }


et coller $cookie, etc. après.
bon boulot Gu1ll4um3r0m41n !

tant que jy pense faudra peut être un patch officiel histoire de valider la soluce de Gu1, etc. La Team Npds attend quoi? :-D

Oui en effet, la fonction première de intval est de convertir une valeur en entier mais elle peut aussi protéger contre les injection SQL quand la valeur à filtrer doit être un entier :p
Je vais donner ma fonction cookiedecode en entier ce sera plus pratique...



function cookiedecode($user) {
global $language;
$stop=false;
if (!empty($_GET)) {
if ($_GET[user]!="") { $stop=true; $user="BAD-GET";}
} else if (!empty($HTTP_GET_VARS)) {
if ($HTTP_GET_VAR[user]!="") { $stop=true; $user="BAD-GET";}
}
if ($user) {
$userX = addslashes(base_64_decode($user));
$cookie = explode(":", $userX);
$cookie[0] = intval($cookie[0]);
if (trim($cookie[1])!="") {
$result = mysql_query("select pass, user_langue from users where uname='$cookie[1]'");
if (mysql_num_rows($result)==1) {
list($pass, $user_langue) = mysql_fetch_row($result);
if (($cookie[2] == md5($pass)) AND ($pass != "")) {
if ($language!=$user_langue) {
mysql_query("UPDATE users set user_langue='$language' where uname='$cookie[1]'");
}
return $cookie;
} else {
$stop=true;
}
} else {
$stop=true;
}
} else {
$stop=true;
}
if ($stop) {
setcookie("user","",0);
unset($user);
unset($cookie);
header("Location: index.php");
}
}
}



faire
$userX = addslashes(base_64_decode($user));
est plus pratique que
$cookie[1] = addslashes($cookie[1]);
$cookie[2] = addslashes($cookie[2]);

:=)

PS:
Sinon, en attendant le patch officiel, il est possible de protéger un site tournant sur NPDS du en désactivant le bloc User ;)

Message édité par : Gu1ll4um3r0m41n / 07-05-2007 19:53

Merci en tous les cas. Je regarde cela dès demain et je publi un correctif au plus vite

A suivre

1er Security Report :
- le addslashes ne me semble pas indispensable compte tenu de la présence du grabber (grab_globals.php)

Je propose donc :
// from Gu1ll4um3r0m41n - 08-05-2007
settype($cookie[0], "integer");
$cookie[1]=urlencode($cookie[1]); // j'en profite

2emem Security Report :

Je propose :
// from Gu1ll4um3r0m41n - 08-05-2007

return (urlencode($realip));

Quand pensez-vous / les deux corrections ne touche que le mainfile. Je peut donc faire une news + un correctif pour sable + une MAJ des versions actuellements disponibles.

A suivre

pourquoi urlencoder l'ip... il suffit simplement de ne pas tenir compte du X_FORWARDED_FOR ni du CLIENT_IP... REMOTE_ADDR est suffisant ;)

Et sinon, pour l'autre truc, meme si le fichier grab_global (qui est d'ailleurs à plus d'inconvénients que d'avantages...) addslashes tout, il faut garder a l'espris que les cookies user et admin sont encodées en base_64. Une chaine encodé en base_64 étant alphanumerique (avec quelques caract speciaux), le fichier grab_global (ou meme les magic_quotes_GPC) n'ont aucun effets sur eux :p
Je pense donc qu'il est utile d'addslasher les cookies une fois décoder ;)

Sinon, pour être un peut critique, je ne recommanderais surement pas NPDS a un ami pour sa sécurité... On dirais que c'est basé sur une vieille (très vieille ><) version de PHP-Nuke -qui est déjà pas top niveau secu- mais que pour chaque faille de PHP-Nuke, NPDS n'as pas été patcher...

Message édité par : Gu1ll4um3r0m41n / 08-05-2007 22:50

Pour Addslashes : vraie / je suis allez trop vite ...

Pour l'IP - la correction porte sur l'ensemble de la fonction et me semble plus simple à mettre en oeuvre pour un novice qui aurait une ancienne version de NPDS que de toucher les IF ...

NPDS est effectivement basé sur une ancienne version de PHP-Nuke / Maintenant nous avons fait des efforts pour améliorer un max de choses et le code n'est pas si ressemblant que cela (sauf pour certains fichiers ammenés à disparaitre dans un proche avenir)

Message édité par : developpeur / 08-05-2007 22:50

Si nécessaire nous pouvons continuer cette conversion en 'privé' : developpeur@npds.org :-)

je n'ais pas de remonter de la team ? / Je publie ?

La sécu c'est ton rayon :b

Même avis que tito.

C'est fait : http://www.npds.org/article.php?sid=1560&mode=thread&order=1&thold=-1