cloudflare / production-saas

(WIP) Example SaaS application built in public on the Cloudflare stack!
https://blog.cloudflare.com/production-saas-intro
MIT License
1.12k stars 49 forks source link

JWT Information leak of User object #23

Open Mecanik opened 2 years ago

Mecanik commented 2 years ago

During inspection and testing of several components, I found many holes and bad practices.

If I have time, I will list them all, but the most important one (and the one that really bugs me) is the fact that you create a JWT token containing ALL the information about the user.

This includes his password, salt... everything. Go to: https://www.javainuse.com/decodeJWT and decode the token you obtain when logging in:

{
    "uid": "...",
    "firstname": "...",
    "lastname": "...",
    "created_at": 1661322119,
    "last_updated": null,
    "password": "d5e838175810db5b0a65a8fad510c701cba81f5cdd0b683a445383301bc37499950e2856a2c39ede8545dd2dac2213bf9166db34c66ade91476c49d51a12d27f",
    "salt": "D7uxWw3hCSMibL20OaRW0agKwZrxcaJ0FyWvHfhq6Pe6TuzugTPMlsscWaqNaPm6JsPGYRxdUraqDg9D-1cM-eKwDHnyeSK8tUouL_S1nzJC8febBOartzJVWlUOoS0I",
    "iat": 1661324521,
    "exp": "166132452186400"
}

I'm not even sure where iat is coming from since it's not in the User object... I guess from worktop/jwt. Anyway, I suggest changing this to a more standard format like: https://www.javainuse.com/jwtgenerator

{
    "Issuer": "Issuer",
    "Issued At": "2022-08-24T06:56:07.654Z",
    "Expiration": "2022-08-24T06:56:07.654Z",
    "Username": "JavaInUse",
    "Role": "Admin"
}

Not sure if any of this matters since this looks like an abandoned project... but anyway, do not use it in production just yet.

Mecanik commented 2 years ago

Just a quick note on the above... I've noticed why the SALT is included in the token (https://github.com/cloudflare/production-saas/blob/e73917a01b3413c14c9ecc47b26b1e6ebdb7f206/src/lib/models/user.ts#L236) and I understand the idea... but I don't think I have seen any app, ever, publicly showing the SALT of a password.

Need another solution to replace that line.

lukeed commented 2 years ago

Email/uid and salt should be the only things contained in the JWT payload. One piece of public identifiable info (immutable) and a “salt” that is a psssword stand in. When a password is changed, so is salt so that all existing JWTs are immediately invalidated.

Mecanik commented 2 years ago

Email/uid and salt should be the only things contained in the JWT payload. One piece of public identifiable info (immutable) and a “salt” that is a psssword stand in. When a password is changed, so is salt so that all existing JWTs are immediately invalidated.

As it stands, everything from User object is included; even the password. Fairly easy to brute force it since we also have the SALT.

On another note, I've checked many JWT aware apps including Netlify, none of them include SALT in the JWT. For the condition payload.salt !== user.salt I'm thinking of an alternative and testing.

lukeed commented 2 years ago

A “salt” alike is the way to go.

I don’t work at CF anymore. Feel free to open a PR to fix the User object inclusion.