Piwigo Bugtracker

Viewing Issue Advanced Details Jump to Notes ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0000261 [Piwigo] security minor always 2006.01.11 13:22 2006.01.21 20:11
Reporter nikrou View Status public  
Assigned To nikrou
Priority normal Resolution fixed Platform
Status closed   OS
Projection none   OS Version
ETA none Fixed in Version 1.5 branch Product Version
  Target Version Product Build
Summary 0000261: Améliorer la sécurité des sessions
Description Jusqu'à la version 1.5.2 les identifiants de sessions sont passés à travers l'url. Cela pose un réel problème potentiel de sécurité. En effet si quelqu'un copie une url complète de phpwebgallery en étant connecté et qu'il la transmet par mail à quelqu'un, alors la personne recevant le mail peut se connecter sur le même compte.
Une solution temporaire est de conseiller de ne pas copier d'url complète.
La solution envisagée est de ne plus passer l'identifiant dans l'url mais de ne se base sur sur les cookies. Une personne pour se connecter devra nécessairement accepter les cookies de sessions.
Steps To Reproduce
Additional Information
Tags No tags attached.
browser any
Database engine and version
PHP version
Web server Apache 1.3.x
Attached Files

- Relationships

-  Notes
(0000558)
plg (manager)
2006.01.11 21:48

Je passe le bug en importance "minor" et pas "major" car il ne se produit pas de manière systématique et ne devient un bug que lorsqu'un utilisateur envoie une URL sans réfléchir à son contenu.
(0000568)
nikrou (developer)
2006.01.15 13:10

intégration dans la branche 1.5 révision 1003
(0000569)
plg (manager)
2006.01.15 13:23
edited on: 2006.01.15 13:28

> intégration dans la branche 1.5 révision 1003

Oulala, malheureux :-( La correction est beaucoup trop impactante pour être faite sur une branche stable. Je pense beaucoup plus sage (et préconisée sur le wiki) l'approche suivante : on améliore en BSF, on corrige les bugs bloquant ou triviaux sur les branches stables + report en BSF.

Donc, a moins de trouver un contre-argument choc, je préferais que tu merges en BSF la r1003 et que tu fasses un retour arrière (un merge r1003:1002 sur la branche 1.5).

As-tu un contre-argument choc ?

(je suis absent jusqu'à mardi 17 janvier 2006, donc je ne pourrai argumenter d'ici là, désolé)

(0000570)
nikrou (developer)
2006.01.15 13:30

Ok. Pas de problème
(0000573)
plg (manager)
2006.01.17 21:53

Petite revue de code de la [Subversion] r1004 (sur trunk cette fois-ci et pas sur la branche 1.5)

Le log de commit ne suit pas les recommandations du wiki : les lignes dépassent 80 caractères, l'identifiant de la demande du bugtracker n'est pas mentionné (difficile d'aller du code vers le bugtracker dans ce cas).

Le fichier install/phpwebgallery_structure.sql a bien été modifié, mais aucun fichier install/db/3-database.php n'a été créé. Le fonctionnement est expliqué sur le wiki, cela permet aux autres développeur de faire automatiquement la mise à jour de leur base. Ce fichier doit comprendre :

- le vidage de la table
- la suppression de la colonne obsolete
- la création de la nouvelle colonne

Les règles de style ne sont pas respectées. Je sais qu'elles ne sont pas encore sur le wiki mais ce n'est pas une raison. Celles qui sont violées :

- les lignes ne doivent pas dépasser 79 caractères
- tabulations interdites
- les accolades de début de block s'ouvrent sur une nouvelle ligne
- le code SQL se construit sur plusieurs lignes (avec indentation)
- utiliser des valeurs booléenes pour les tests et pas des entiers

Je sais qu'une discussion est en cours sur le changement de certaines règles, mais pour le moment, rien n'est statué.

Dans le fichier include/config_default.inc.php, le commentaire de chaque paramètre doit commencer par le nom du paramètre, suivi de ":" et la description.

Toujours dans include/config_default.inc.php, pour dire true, il ne faut pas utiliser 1 et 0 pour false. Il faut vraiment utiliser true et false. C'est beaucoup plus lisible (et cela fait parti des conventions de codes).

Désolé pour ces quelques remarques, rien de personnel évidemment. J'avais prévenu que je ferai une "revue de code". Je t'encourage à faire de même sur le code que je commiterai prochainement.
(0000577)
plg (manager)
2006.01.18 22:17

A propos de la [Subversion] r1007

Pour faire plus propre, je remplacerai

if (isset($conf['session_use_cookies']))
{
  ini_set('session.use_cookies', $conf['session_use_cookies']);
}
if (isset($conf['session_use_only_cookies']))
{
  ini_set('session.use_only_cookies', $conf['session_use_only_cookies']);
}
if (isset($conf['session_use_trans_sid']))
{
  ini_set('session.use_trans_sid', intval($conf['session_use_trans_sid']));
}
if (isset($conf['session_name']))
{
  ini_set('session.name', $conf['session_name']);
}

par

ini_set('session.use_cookies', $conf['session_use_cookies'] ? 1 : 0);
ini_set('session.use_only_cookies', $conf['session_use_only_cookies'] ? 1 : 0);
ini_set('session.use_trans_sid', $conf['session_use_trans_sid'] ? 1 : 0);
ini_set('session.name', $conf['session_name']);

(sur le bugtracker, la police n'est pas monospace, mais il y a un alignement...)

Dans les requêtes SQL, FROM, SET et autre VALUES sont sur une nouvelle ligne, après une indentation. Toujours dans le SQL, une requête se termine par un ";" sur une nouvelle ligne (en activation l'option show_queries, tu vas comprendre pourquoi tout ceci est important).

Toute fonction mérite son commentaire :-)
(0000578)
nikrou (developer)
2006.01.21 20:11

Correction sur le code tant sur la forme que sur le fond. [Subversion] r1010

- Issue History
Date Modified Username Field Change
2006.01.11 13:22 nikrou New Issue
2006.01.11 13:22 nikrou browser => any
2006.01.11 13:22 nikrou Web server => Apache 1.3.x
2006.01.11 13:23 nikrou Status new => assigned
2006.01.11 13:23 nikrou Assigned To => nikrou
2006.01.11 13:23 nikrou Issue Monitored: nikrou
2006.01.11 13:24 nikrou Issue End Monitor: nikrou
2006.01.11 20:52 plg Issue Monitored: plg
2006.01.11 21:48 plg Note Added: 0000558
2006.01.11 21:48 plg Severity major => minor
2006.01.15 13:10 nikrou Note Added: 0000568
2006.01.15 13:11 nikrou Status assigned => resolved
2006.01.15 13:11 nikrou Fixed in Version => 1.5 branch
2006.01.15 13:11 nikrou Resolution open => fixed
2006.01.15 13:23 plg Note Added: 0000569
2006.01.15 13:28 plg Note Edited: 0000569
2006.01.15 13:30 nikrou Status resolved => feedback
2006.01.15 13:30 nikrou Resolution fixed => reopened
2006.01.15 13:30 nikrou Note Added: 0000570
2006.01.15 13:31 nikrou Status feedback => resolved
2006.01.15 13:31 nikrou Resolution reopened => fixed
2006.01.17 21:53 plg Note Added: 0000573
2006.01.18 22:17 plg Note Added: 0000577
2006.01.21 20:11 nikrou Status resolved => closed
2006.01.21 20:11 nikrou Note Added: 0000578


Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Contact
Powered by Mantis Bugtracker