Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sécurité] Vulnérabilité de type CSRF pour changer le password #554

Closed
PaulSec opened this issue Jul 30, 2014 · 8 comments
Closed

[Sécurité] Vulnérabilité de type CSRF pour changer le password #554

PaulSec opened this issue Jul 30, 2014 · 8 comments

Comments

@PaulSec
Copy link

PaulSec commented Jul 30, 2014

Hello,

Il semblerait que FreshRSS soit vulnérable à une attaque de type CSRF (Cross Site Request Forgery).
En effet, il est possible de créer un formulaire du type:

<form action="hxxps://rss.monflux.fr/i/?c=users&a=auth" id="exploit">
<input type="hidden" name="passwordPlain" value="omgwtfbbq">
<input type="hidden" name="mail_login" value="">
<input type="hidden" name="auth_type" value="form">
<input type="hidden" name="anon_access" value="1">
<input type="hidden" name="token" value="">
</form>

Et de l'auto-soumettre à l'aide de JavaScript.

HTMLFormElement.prototype.submit.call($('#exploit')[0]);

L'attaque se base sur le fait que l'utilisateur est connectée sur FreshRSS & navigue sur le Net en même temps (ou visualise une news qui comprend du code malicieux).
Dès chargement de la page, le mot de passe de l'utilisateur courant est changé.

@Alkarex Alkarex added this to the 0.9.0 milestone Jul 30, 2014
@Alkarex
Copy link
Member

Alkarex commented Jul 30, 2014

Je pense que le problème est présent pour toutes les actions, pas seulement pour changer le mot de passe.
Il faudrait s'assurer que toutes les actions critiques sont bien en POST, et pour toutes les actions POST, exiger un paramètre secret supplémentaire.

Alkarex added a commit that referenced this issue Aug 1, 2014
#554
Also edited the error controler to use the log message passed in
Minz_Error::error().
@Alkarex
Copy link
Member

Alkarex commented Aug 1, 2014

Je viens d'ajouter le contrôle du domaine dans l'entête HTTP Referer pour toutes les requêtes POST (hors API) https://github.com/marienfressinaud/FreshRSS/blob/35be1769de28df3fff1a26e40d1d6b1e587a2847/app/FreshRSS.php#L9.
Cela ne suffit pas, mais devrait déjà bien réduire la sévérité du problème, en supprimant bon nombre de possibilités de XSRF.

Si vous en avez la possibilité, merci de vérifier que cela bloque bien votre faille de sécurité initiale (j'ai bien sûr testé de mon côté).

Voilà une capture d'écran qui s'affichera sur l'écran de l'attaqué dans certains cas :

403

Il faudra rester attentif à ce que cela ne casse pas certaines configurations ésotériques.

@Alkarex Alkarex modified the milestones: 0.9.0, 0.8.0 Aug 1, 2014
@Alkarex Alkarex self-assigned this Aug 1, 2014
@Alkarex
Copy link
Member

Alkarex commented Aug 1, 2014

@marienfressinaud As-tu en tête des actions importantes qui ne sont pas en POST ?

@PaulSec
Copy link
Author

PaulSec commented Aug 1, 2014

Très bien, surtout que c'est une recommandation faite par OWASP

Alkarex added a commit that referenced this issue Aug 1, 2014
@Alkarex
Copy link
Member

Alkarex commented Aug 1, 2014

Quelqu'un sait-il si des navigateurs implémentent l'entête Origin et si oui, lesquels ?
https://wiki.mozilla.org/Security/Origin

This was referenced Aug 3, 2014
@marienfressinaud
Copy link
Member

@PaulSec > merci Paul ! :)

@Alkarex > à priori on a fait attention à ce que toutes les actions importantes passent par des POST… niveau configuration / paramétrage en tout cas. Maintenant il reste les actions comme "marquer comme lu" et "favoris". Si un id n'est pas forcément facile à deviner, il reste la question du bouton "tout marquer comme lu" qui peut être plus problématique pour certains.

@Alkarex
Copy link
Member

Alkarex commented Aug 8, 2014

La conversation pour une meilleure méthode basée sur un token continue dans #570 , et celle sur le HTTP Referer dans #565

@Alkarex Alkarex closed this as completed Aug 8, 2014
Alkarex added a commit that referenced this issue Aug 30, 2014
Now tests also for the scheme and port, which must be identical to the
ones in the referer.

#565 (comment)
#554
Alkarex referenced this issue Nov 4, 2014
If needed, we may re-introduce the check for scheme with proper support
for proxy
#565 (comment)
@Alkarex
Copy link
Member

Alkarex commented Aug 13, 2016

Après une période où la vérification du HTTP Referer nous a bien servi, je viens de passer à une méthode utilisant un token anti CSRF #1210 afin de pouvoir anonymiser les liens sortants avec <meta name="referrer" content="never" /> #955
@PaulSec Tests de la branche /dev de FreshRSS bienvenus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants