Anomalie #1647
fermé
GroupsController::doEdit n'est pas protégée : possibilité de soumettre des modifications non autorisées
100%
Description
Un responsable de groupe peut modifier n'importe quel groupe, même ceux dont il n'est pas responsable. L'interface utilisateur n'affiche certes pas les groupes dont il n'est pas responsable, cependant la route GroupsController::doEdit ne vérifie pas que l'utilisateur qui soumet les données du formulaire est bien un gestionnaire du groupe.
Il est donc possible de s'identifier (avec un compte qui est gestionnaire d'un groupe) puis de soumettre une requête qui porte sur un autre identifiant de groupe que ceux autorisés. La requête est acceptée.
Dans le template, l'interface utilisateur est gardée par le booléen "can_edit" défini dans le gabarit. Cependant le code qui traite la requête POST ne fait pas la même modification.
Cette faille me semble uniquement exploitable par les comptes ayant un statut responsable de groupe pour au moins un groupe parmi tous ceux qui existent. Les autres utilisateurs (non "groupmanager") sont bloqués par les ACLs dans core_acls.php.
Je pense que le comportement correct serait de faire, en tête de GroupsController::doEdit, la même vérification que le "can_edit" du template. Je pense essayer de rédiger un patch.
Fichiers
Mis à jour par Florian Hatat il y a plus de 2 ans
En regardant d'un peu plus près le code de GroupsController, je pense qu'aucun accès n'est vraiment protégé dans le code PHP. On peut aussi soumettre les requêtes Ajax (pour la lecture) sur des identifiants de groupes dont on n'est ni membre ni responsable.
Mis à jour par Florian Hatat il y a plus de 2 ans
- Fichier group_permissions.patch group_permissions.patch ajouté
Voici une tentative de patch qui m'a l'air de fonctionner (sur Galette 0.9.6.1 ; pour le template twig, j'ai modifié en aveugle, n'ayant pas facilement d'instance de test pré-1.0.0 sous la main).
J'ai ajouté une méthode Group::canEdit($login) sur le modèle de ce qui existe pour Adherent::canEdit($login). Un appel à cette méthode garde getGroup (affichage du détail d'un groupe) et doEdit. Le reste me semble protégé via les ACL mais il n'est pas inutile de me relire, je ne suis pas très sûr de moi.
J'ai notamment des doutes sur la méthode "reorder", qui apparemment permet de réordonner tous les groupes même si l'on n'est pas manager sur tous. De plus, doAdd est réservée aux admin/staff mais doDelete est ouverte à tous les managers. Je ne sais pas si c'est cohérent et/ou désiré.
Mis à jour par Johan Cwiklinski il y a plus de 2 ans
Aucune issue de sécurité ne doit être ouverte de manière publique !
J'ai passé en privé (aucune idée de ce que ça implique), je regarderai ça quand j'en aurai le temps.
Mis à jour par Florian Hatat il y a plus de 2 ans
Navré pour le mode public, je n'ai pas trouvé de procédure spécifique autre que le bugtracker (j'ai peut-être mal cherché), cependant il me semble qu'en tant qu'utilisateur standard, Redmine ne me donne pas accès au réglage public/privé. Le mieux que j'ai trouvé a été de l'étiqueter avec le champ "Catégorie".
Pour être un peu plus précis sur la (légère) sévérité :- je crois que ce n'est exploitable que par des utilisateurs identifiés, eux-mêmes ayant un statut responsable de groupe ;
- il peut y avoir des fuites de données (responsables ayant accès à des données d'autres adhérents alors qu'ils n'auraient pas dû) ;
- pas d'élévation de privilège interne à Galette ;
- pas d'exécution de code arbitraire coté serveur.
Mis à jour par Johan Cwiklinski il y a plus de 2 ans
Florian Hatat a écrit (#note-5):
Navré pour le mode public, je n'ai pas trouvé de procédure spécifique autre que le bugtracker (j'ai peut-être mal cherché), cependant il me semble qu'en tant qu'utilisateur standard, Redmine ne me donne pas accès au réglage public/privé. Le mieux que j'ai trouvé a été de l'étiqueter avec le champ "Catégorie".
[...]
Outre la sévérité ou la pertinence, les rapports de sécurité ne doivent pas être publics avant d'avoir été corrigés ; pas que pour Galette. Il me semblait avoir mis sur la doc ou le site une adresse dédiée, mais force est de constater que ce n'est pas (ou plus...) le cas. Dans ces cas là, l'option à retenir c'est de contacter l'équipe par un biais privé (courriel par exemple) ou de demander comment procéder sur un canal public en restant vague - il ne faut surtout pas divulguer la manière de reproduire la faille ;)
Sinon en effet, l'impact est relativement limité, puisqu'il faut déjà être responsable de groupe - et savoir comment faire. J'ai pas mal de boulot côté pro, et de petites vacances en vue ; je regarderai le patch proposé dès que possible, je reviens vers toi.
Merci :)
Mis à jour par Florian Hatat il y a presque 2 ans
- Statut changé de Nouveau à Résolu
- % réalisé changé de 0 à 100
Appliqué par commit c2e1d5376fd471a37c8ac978098f72816c2169ac.
Mis à jour par Johan Cwiklinski il y a plus d'un an
- Statut changé de Résolu à Fermé
- Version cible mis à 1.0.0