ppy / osu-web

the browser-facing portion of osu!
https://osu.ppy.sh
GNU Affero General Public License v3.0
970 stars 381 forks source link

Cookies not secure #5786

Closed robinknaapen closed 4 years ago

robinknaapen commented 4 years ago

When testing API v2 calls I notices some cookies being set, not a big deal. Just a part I did not expect to see. Next thing I noticed that all cookies are set without Secure. I also noticed that on the osu! website all cookies are not Secure.

Is this as intended? Because this is vulnerable to MITM. Even if you store sessions/tokens per IP base. This should be considered a vulnerability from a security perspective.


Edit:

nanaya commented 4 years ago

It's not actually vulnerable to MITM in sane browsers due to HSTS preload.

But probably a good thing to set anyway, in .env:

SESSION_SECURE_COOKIE=true

And make sure we're not kicked out from preload list by fixing some problems: https://hstspreload.org/?domain=ppy.sh

Update: keeping high priority because we're at risk being removed from the list.

robinknaapen commented 4 years ago

@nanaya makes sense. I should've dug a little bit deeper before making such a bold claim... Even so I'm glad it's still being added.

robinknaapen commented 4 years ago

One more question: why is osu_session exposed to ppy.sh and not exclusivly to osu.ppy.sh

nanaya commented 4 years ago

notification websocket endpoint is on different subdomain and it needs cookies for authentication

peppy commented 4 years ago

HSTS fixed and env updated 👍