WIP: empêcher le contrôle des redirections sur les mails d'enregistrement et de réinitialisation de mot de passe (#76835) #44

bdauvergne · 2023-04-20T16:46:11+02:00

bdauvergne commented

2023-04-20 16:46:11 +02:00

No description provided.

pmarillonnet commented

2023-04-20 17:09:46 +02:00

Ce n’est pas ce que relève le rapport d’audit.
La next_url arbitraire se fait sur le POST /password/reset/confirm/.
Effectivement en testant de mon coté, j’arrive à la modifier sans que ça fâche authentic.

Ce n’est pas ce que relève le rapport d’audit. La next_url arbitraire se fait sur le `POST /password/reset/confirm/`. Effectivement en testant de mon coté, j’arrive à la modifier sans que ça fâche authentic.

pmarillonnet commented

2023-04-20 17:15:20 +02:00

Ce n’est pas ce que relève le rapport d’audit.
La next_url arbitraire se fait sur le POST /password/reset/confirm/.
Effectivement en testant de mon coté, j’arrive à la modifier sans que ça fâche authentic.

Et, chose que je n’ai pas testée, le rapport semble indiquer qu’on peut utiliser la next-signature de ce lien généré pour la réinitialisation de mot de passe, pour forger une url frauduleuse de création de compte.

> Ce n’est pas ce que relève le rapport d’audit. > La next_url arbitraire se fait sur le `POST /password/reset/confirm/`. > Effectivement en testant de mon coté, j’arrive à la modifier sans que ça fâche authentic. Et, chose que je n’ai pas testée, le rapport semble indiquer qu’on peut utiliser la next-signature de ce lien généré pour la réinitialisation de mot de passe, pour forger une url frauduleuse de création de compte.

pmarillonnet requested changes 2023-04-20 17:18:11 +02:00

pmarillonnet left a comment

C’est bien d’avoir ce test mais ce n’est pas ce que semble relever le rapport.

pmarillonnet commented

2023-04-20 17:32:45 +02:00

Arf, le rapport est pas clair, je ne suis pas sur d’avoir capté. En relisant j’ai l’impression que rien à voir avec du CSRF en fait, le rapport suppose que l’attaquant modifie le payload POST d’une demande de ré-initialisation vraiment soumise par l’usager cible de l’attaque, auquel cas la vérification CSRF n’est pas invalidée mais il n’y a pas de vérif sur la next_url initiale (?)
Ça me paraît un peu gros cette affaire.

Arf, le rapport est pas clair, je ne suis pas sur d’avoir capté. En relisant j’ai l’impression que rien à voir avec du CSRF en fait, le rapport suppose que l’attaquant modifie le payload POST d’une demande de ré-initialisation vraiment soumise par l’usager cible de l’attaque, auquel cas la vérification CSRF n’est pas invalidée mais il n’y a pas de vérif sur la next_url initiale (?) Ça me paraît un peu gros cette affaire.

fpeters commented

2023-04-20 18:30:21 +02:00

Perso j'avais noté dans https://dev.entrouvert.org/issues/76816#note-7 :

cf leur rapport, il y a deux temps :

1/ l'attaqueur fait la demande de réinitialisation de mot de passe, il contrôle son ordi il envoie le next_url qu'il souhaite
2/ la victime reçoit un mail ok c'est bien toulouse j'ai confiance je complète le formulaire, oh non j'arrive sur le site des méchants.

C'est en tout cas ce que j'ai lu de leur scénario d'exploitation : (page 26)

Un attaquant envoie un lien malveillant à un utilisateur de l’application.
L’utilisateur clique sur ce lien et est alors redirigé vers un site contrôlé par l’attaquant.

Perso j'avais noté dans https://dev.entrouvert.org/issues/76816#note-7 : > cf leur rapport, il y a deux temps : > > 1/ l'attaqueur fait la demande de réinitialisation de mot de passe, il contrôle son ordi il envoie le next_url qu'il souhaite > 2/ la victime reçoit un mail ok c'est bien toulouse j'ai confiance je complète le formulaire, oh non j'arrive sur le site des méchants. C'est en tout cas ce que j'ai lu de leur scénario d'exploitation : (page 26) > Un attaquant envoie un lien malveillant à un utilisateur de l’application. > L’utilisateur clique sur ce lien et est alors redirigé vers un site contrôlé par l’attaquant.

bdauvergne commented

2023-04-20 22:27:20 +02:00

Ok je comprends mieux.

bdauvergne changed title from ~~tests: check open redirection is impossible for /password/reset/ (#76835)~~ to WIP: tests: check open redirection is impossible for /password/reset/ (#76835)

2023-04-20 22:34:18 +02:00

bdauvergne changed title from ~~WIP: tests: check open redirection is impossible for /password/reset/ (#76835)~~ to WIP: empêcher le contrôle des redirections sur les mails d'enregistrement et de réinitialisation de mot de passe (#76835)

2023-04-20 22:34:57 +02:00

bdauvergne force-pushed wip/76835-Verifier-la-possibilite-de-redir from c8c50ea028 to bd8c937773

2023-04-20 23:12:41 +02:00

Compare

bdauvergne force-pushed wip/76835-Verifier-la-possibilite-de-redir from bd8c937773 to 3dff2974d2

2023-04-21 08:40:07 +02:00

Compare

bdauvergne closed this pull request

2023-11-09 14:25:06 +01:00

gitea/authentic/pipeline/head This commit looks good

Details

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

3 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/authentic#44