Annonce

Écrire une réponse

Veuillez écrire votre message et l'envoyer

Cliquez dans la zone sombre de l'image pour envoyer votre message.

Retour

Résumé de la discussion (messages les plus récents en premier)

nicolas
2006-07-21 12:49:44

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
2006-07-21 11:53:31

nicolas a écrit:

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 ? :-)

Heu, je n'ai pas encore assez étudié le sujet! Donc pour le moment, je m'abstiens...

nicolas
2006-07-21 10:14:42

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 ? :-)

rub
2006-07-21 09:37:49

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%

nicolas
2006-07-21 09:02:08

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!

rvelices
2006-07-20 23:46:52

nicolas a écrit:

Ce n'est pas une bonne idée de systématiquement utilisé addslashes() sur toutes les données entrantes (get, post ou cookie).

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...

nicolas
2006-07-20 21:49:33

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:

Code:

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:

Code:

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:

Code:

--- 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 ?

Pied de page des forums

Propulsé par FluxBB

github twitter newsletter Faire un don Piwigo.org © 2002-2024 · Contact