Devant un tel enthousiasme je vais commencer par intégrer cela dans le tronc.

Finalement j'ai aussi fait le report dans la branche 1.6

rub a écrit:

nicolas a écrit:

J'ai crée un post dans le forum pour vous prévenir et pour que l'on prenne ensemble de nouvelles bonnes habitudes!

BugTracker+forum pour les sujets "délicats" (ou non mineures) => Approuvé à 100%

ok mais puis-je avoir ton avis sur la question ? :-)

rvelices a écrit:

Je pense qu'aujourd'hui c'est fait pour des raisons de securite en common.inc.php dans tous les cas. Je n'ai pas bien compris si tu veux le virer de ce fichier, mais dans ce cas il faut faire un passage vraiment partout pour s'assurer qu'on ne peut pas injecter du sql...

J'en suis bien conscient et c'est pourquoi la correction que je propose fonctionne même si on laisse cela. J'ai crée un post dans le forum pour vous prévenir et pour que l'on prenne ensemble de nouvelles bonnes habitudes!

Bonjour à tous,
suite à ce rapport de bug je me suis demandé pourquoi il y avait le problème. En fait la gestion des quotes n'est pas très bien faite.
Ce n'est pas une bonne idée de systématiquement utilisé addslashes() sur toutes les données entrantes (get, post ou cookie). En particulier il ne faut pas le faire pour les données entrantes dans la base sous peine de voir les quotes backslashées deux fois.

Je propose de corriger le problème en créant une fonction pwg_quotemeta() qui pourrait ressembler à cela:

function pwg_quotemeta($value) {
   if (get_magic_quotes_gpc()) {
       $value = stripslashes($value);
   }
   if (function_exists('mysql_real_escape_string'))
   {
     $value = mysql_real_escape_string($value);
   }
   else
   {
     $value = mysql_escape_string($value);
   }
   return $value;
}

Après pour résoudre le bug, j'ajouterais deux fonctions:

function pwg_stripslashes($value) 
{
  if (get_magic_quotes_gpc())
  {
    $value = stripslashes($value);
  }
  return $value;
}

function pwg_addslashes($value) 
{
  if (!get_magic_quotes_gpc())
  {
    $value = addslashes($value);
  }
  return $value;
}

Il suffit alors d'appliquer le patch suivant au fichier tags.php:

--- admin/tags.php  (rvision 1482)
+++ admin/tags.php  (copie de travail)
@@ -149,20 +149,12 @@
 
 if (isset($_POST['add']) and !empty($_POST['add_tag']))
 {
-  if (function_exists('mysql_real_escape_string'))
-  {
-    $tag_name = mysql_real_escape_string($_POST['add_tag']);
-  }
-  else
-  {
-    $tag_name = mysql_escape_string($_POST['add_tag']);
-  }
-
+  $tag_name = $_POST['add_tag'];
   // does the tag already exists?
   $query = '
 SELECT id
   FROM '.TAGS_TABLE.'
-  WHERE name = \''.$tag_name.'\'
+  WHERE name = \''.pwg_quotemeta($tag_name).'\'
 ;';
   $existing_tags = array_from_query($query, 'id');
 
@@ -173,7 +165,7 @@
       array('name', 'url_name'),
       array(
         array(
-          'name' => $tag_name,
+          'name' => pwg_quotemeta($tag_name),
           'url_name' => str2url($tag_name),
           )
         )
@@ -183,7 +175,7 @@
       $page['infos'],
       sprintf(
         l10n('Tag "%s" was added'),
-        $tag_name
+        pwg_stripslashes($tag_name)
         )
       );
   }

Il faudrait utiliser le même système partout où l'on fait des requêtes.

Qu'en pensez-vous ?