pulibrary / figgy

Valkyrie-based digital repository backend.
Other
36 stars 4 forks source link

Use omniauth-rails_csrf_protection gem to resolve CVE-2015-9284 #3132

Open eliotjordan opened 5 years ago

eliotjordan commented 5 years ago

How are we feeling about CVE-2015-9284? I've been watching the omni-auth repo comments, but no fix is forthcoming.

There is a gem that mitigates this vulnerability: https://github.com/cookpad/omniauth-rails_csrf_protection

Implementation: https://github.com/brave-intl/publishers/pull/1942

Is this worth trying, or should we wait for an official fix?

jrgriffiniii commented 5 years ago

I was trying to be optimistic, but https://github.com/omniauth/omniauth/pull/809 doesn't appear to be moving ahead. DCE is using that Gem for repository projects: https://github.com/curationexperts/laevigata/pull/1907

escowles commented 5 years ago

My understanding of this vulnerability is that it requires an OAuth provider with another CSRF vulnerability in order to exploit the vulnerability. That is, the OAuth provider's vulnerability would be used to inject forged headers into Figgy.

Based on that understanding, I think our exposure here is very small. We do use OAuth for authorizing Google Drive support (and we also use it as the main auth mechanism in cicognara). But the overwhelming majority of our users aren't using Google Drive support at all, and those that do use it don't need to re-auth all the time. So I think the circumstances where this attack could even arise are pretty rare in Figgy.

If that is all true, then I'm fine waiting for an official fix. I'm open to using that external CSRF fix, but I'd want to know why Omniauth isn't just merging the fix.

eliotjordan commented 3 years ago

Implementation in Pulmap: https://github.com/pulibrary/pulmap/pull/910