NextINpact / nextinpactv7

7 stars 0 forks source link

Stockage inadapté du token de session (non sécurisé) #58

Open thibaut-decherit opened 4 years ago

thibaut-decherit commented 4 years ago

J'ai déduit que le token de session était inpact-token étant donné qu'en le supprimant ça me déconnecte. Si je fais erreur, ne pas tenir compte de cette issue.

Le token de session inpact-token est stocké en local storage. Le local storage n'est PAS adapté pour le stockage de données sensibles/privées/confidentielles car il est accessible à TOUT JavaScript exécuté sur la page, rendant possible le vol de son contenu par exemple en cas de XSS, ou si du code malveillant se retrouve dans une librairie que vous utilisez.

C'est d'autant plus un problème que vous n'avez pas de Content Security Policy pour limiter par exemple vers quels domaines le JS présent sur vos pages peut envoyer des données (via la directive connect-src) : ainsi, du JS malveillant pourrait lire le token de session et le faire fuiter vers un domaine tiers via une requête AJAX. Dans la même veine, du JS malveillant pourrait faire une "fausse" requête GET vers un domaine tiers en attachant le token de session à l'URL. Par exemple en ajoutant au DOM :

Voire via une requête POST en modifiant l'attribut action d'un formulaire déjà présent sur le site (serait bloqué par la directive CSP form-action). Bref, vous saisissez l'idée !

Il est préférable de stocker un token d'authentification dans un cookie avec le flag HttpOnly, car le navigateur refusera au JS l'accès aux cookies ayant ce flag. Le bonus étant que vu que c'est un cookie, le navigateur se chargera nativement de l'envoyer avec les requêtes, donc ce n'est pas un problème de ne pas pouvoir y accéder en JS. A moins que vous deviez envoyer ce cookie à un autre domaine que celui qui l'a créé, mais ça a l'air d'être destiné (entre autre ?) au sous-domaine api-v1.nextinpact.com donc ça devrait le faire avec les bons réglages au moment de la création.

D'ailleurs vous semblez passer ce token dans des URLs, comme https://api-v1.nextinpact.com/notifications/negotiate?signalrtoken=, c'est pas super propre de faire de l'auth via l'URL... Peut-être un problème de conception de ce côté.

Pour aller plus loin concernant le local storage (et la CSP) :

Exagone313 commented 4 years ago

De plus, à propos de JWT :

pehadavid commented 4 years ago

@thibaut-decherit merci, je vais étudier tout ça. En soit je comprends tout à fait la crainte et la faille potentielle et ça mérite vraiment de se pencher sur le sujet. Si tout cela est pertinent, on switchera vers un stockage sur un cookie.