Project

General

Profile

Actions

Anomalie #1647

closed

GroupsController::doEdit n'est pas protégée : possibilité de soumettre des modifications non autorisées

Added by Florian Hatat almost 2 years ago. Updated 7 months ago.

Status:
Fermé
Priority:
Haut
Category:
Security
Target version:
Start date:
07/18/2022
Due date:
% Done:

100%

Estimated time:
Version utilisée:

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.


Files

group_permissions.patch (4.02 KB) group_permissions.patch Test de permission sur les vues doEdit et getGroup Florian Hatat, 07/18/2022 04:02 PM
Actions #1

Updated by Florian Hatat over 1 year ago

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.

Actions #2

Updated by Florian Hatat over 1 year ago

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

Actions #3

Updated by Johan Cwiklinski over 1 year ago

  • Private changed from No to Yes
Actions #4

Updated by Johan Cwiklinski over 1 year ago

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.

Actions #5

Updated by Florian Hatat over 1 year ago

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.
Actions #6

Updated by Johan Cwiklinski over 1 year ago

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

Actions #7

Updated by Florian Hatat over 1 year ago

  • Status changed from Nouveau to Résolu
  • % Done changed from 0 to 100
Actions #8

Updated by Johan Cwiklinski over 1 year ago

  • Private changed from Yes to No
Actions #9

Updated by Johan Cwiklinski 7 months ago

  • Status changed from Résolu to Fermé
  • Target version set to 1.0.0
Actions

Also available in: Atom PDF