Closed plr0man closed 6 years ago
I'm a big fan of clear baseline priorities, so from the start I would be against solution number 2. :)
Looking at the VRT, the only entry about OAuth seems to be the one you mentioned. I agree the missing state parameter can often lead to account takeover, but so can other things, for example the lack/insufficient of validation of a redirect_uri, etc.
Maybe something like this would be appropriate:
I marked the first one P2, because I'm not sure it should be P1; I think there's quite a bit of discussion that is needed regarding "Account Takeover" (a takeover with zero victim involvement is very different from a takeover through a CSRF or OAuth misconfiguration, in my opinion), but that is perhaps better handled as a separate issue.
I am definitely in favor of keeping Server Security Misconfiguration > OAuth Misconfiguration > Missing State Parameter
. Updating the name to Missing/Broken State Parameter
could be helpful though.
We need Server Security Misconfiguration > OAuth Misconfiguration > Missing State Parameter
for two reasons:
redirect_uri
entry and more.I like where you're going with this one @truemongo P2 - Server Security Misconfiguration > OAuth Misconfiguration > Account Takeover
, but have you considered OAuth misconfiguration issues that lead to account takeover without the need for victim interaction? Or are you thinking of applying Broken Authentication and Session Management > Authentication Bypass
for such cases?
There's also been an idea for moving the state parameter entry under CSRF. I am personally leaning towards entries calling out particular misconfigurations under current subcategory.
Let's put a more comprehensive solution on the backburner though, as we're nearing a new release and this issue needs a quick fix with P4 being inaccurate. This would mean "Varies" (P5-P2) instead of current P4:
null – Server Security Misconfiguration > OAuth Misconfiguration > Missing/Broken State Parameter
Looks like we might have a few extra days before the next release, so let's consider one more option as a combination of the ideas so far:
P2 - Server Security Misconfiguration > OAuth Misconfiguration > Account Takeover
null – Server Security Misconfiguration > OAuth Misconfiguration > Insecure Redirect URI
null – Server Security Misconfiguration > OAuth Misconfiguration > Missing/Broken State Parameter
For all those cases where state
or redirect_uri
don't lead to account takeover, the context based ("Varies") entries would apply, otherwise the P2 variant could be the entry of choice.
Please share any other types of OAuth specific misconfigurations that don't always lead to account takeover so we can provide a more comprehensive selection.
Are there other issues with Insecure Redirect URI
that can be categorized as P3 instead of P4 when that misconfiguration is used as an open redirect? What about P5? If not, maybe it is better to categorize it as P4.
Another question regarding this comment @plr0man :
I like where you're going with this one @truemongo P2 - Server Security Misconfiguration > OAuth Misconfiguration > Account Takeover, but have you considered OAuth misconfiguration issues that lead to account takeover without the need for victim interaction?
If victim interaction is so important here, would it be possible to chain an open redirect and force the victim to visit the malicious site with a CSRF ready to execute. Is that considered P2?
Please share any other types of OAuth specific misconfigurations that don't always lead to account takeover so we can provide a more comprehensive selection.
What about race conditions? After obtaining code
in Auth 2.0, generate several access_token
and check whether each of them is valid. After revocation, are you invalidating every single access_token
?. The same applies in the case of refresh_token
.
To clarify the redirect_uri
variant would be offered for situations where you can't steal the auth token via callback URL (P2 - Server Security Misconfiguration > OAuth Misconfiguration > Account Takeover
), but there might be other problems like too liberal validation of redirect_uri
that allows callbacks to subdomains or OAuth code
disclosure via open redirect.
What about race conditions? After obtaining code in Auth 2.0, generate several access_token and check whether each of them is valid. After revocation, are you invalidating every single access_token?. The same applies in the case of refresh_token.
Sky is the limit, context is king:)
Current VRT entry
P4 – Server Security Misconfiguration > OAuth Misconfiguration > Missing State Parameter
has a baseline priority of P4. Considering that this vulnerability can have various impact and frequently results in account takeover this default does not seem accurate.I see two solutions:
All feedback is welcome