python/blurb_it

CSRF vulnerability

ewjoachim opened this issue · 4 comments

I have discussed with @Mariatta privately, and I was told to open a public issue, so here it is.

The view that launches blurb_it doesn't protect itself against CSRF attacks.
This means if I have a session on https://blurb-it.herokuapp.com/, and I visit, say, attacker.com which contains the following script:

fetch("https://blurb-it.herokuapp.com/add_blurb", {
    "credentials": "include",
    "headers": {
        "Content-Type": "application/x-www-form-urlencoded",
    },
    "body": "bpo_number=1&pr_number=1&section=Security&news_entry=yay",
    "method": "POST",
    "mode": "cors"
});

then a blurb would be silently created.

I guess that this it not critical, given it would be reviewed before it's merged into CPython but still, that could make things strange for the victim who would have a blurb on their PR that they didn't create.

Mitigations would include:

  • Adding a CSRF token: a random string stored in the session, and added as <input type="hidden> in the form. It's value would be checked on POST and if the form and the session differ, the POST would be rejected. This requires code, but would be enough on itself.
  • Making the aiohttp session cookie SameSite attribute be Lax. This is discussed upstream in aiohttp & aiohttp-session: the option to pass a value for SameSite has been merged but not released yet in aiohttp and a ticket for making this available in aiohttp-session is openned. This would solve the problem on modern browsers, but old browsers would still be vulnerable. That being said, the target users of blurb_it most probably use modern browsers.

So there's an aiohttp-csrf package that's made to work with aiohttp-session. In normal circumstances, I'd be delighted but:

  • It's 0.0.2, which might be early stage to use
  • It's been written 3 years ago, there was no movement up until a few days ago
  • The code corresponding to the version currently on pypi lives in an unmerged PR that is based on another unmerged PR.

All in all, it seems we cannot use that yet.

Thanks @ewjoachim.

@asvetlov @webknjaz any advice for handling csrf in aiohttp?

(I've gone ahead and reimplemented a simple CSRF check, but I'd be more than happy to get a check from someone else !)

That sounds about right. It's not a part of the framework so either use third-party or reimplement it.