-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Open redirect vulnerability when prompt=none
#61
Comments
@meagar thanks again :) I've prepared a PR in #66 to catch this, I would appreciate it if you could take a look! Also, not sure if this is serious enough to warrant a CVE? Let me know if you have experience with that, otherwise I'll look into it.
I'm not sure if the 422 error should have precedence, since we're already returning the OIDC/OAuth error. The code in #66 currently returns a 401. |
Released with version 1.5.4 |
Sorry, I've been away from the Internet for personal reasons. I don't know if this warrants a CVE, but it'd be fun for me to create one. Would you be OK with that? I think it's wise to publicly disclose this problem, since it's a very real attack vector. |
@meagar thanks, but I was curious as well and requested one through iwantacve.org :) I got an automated reply a few weeks ago and just inquired again about the current state. I also see now they have an official form online at https://cveform.mitre.org/, it used to redirect to a Google form so maybe that's why my request got lost. |
@meagar ok had to submit a new request and got a CVE assigned now, see https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-9837 I credited you as the discoverer but looks like it's not displayed anywhere, sorry ;-) |
tl;dr Doorkeeper OIDC can be provoked to blindly redirect the browser to whatever site is sent in
redirect_uri
paramter whenprompt=none
.Given an otherwise valid authorization attempt with
prompt=none
, if an error can be provoked, the user is redirected to whatever redirect URI is passed in through theredirect_uri
parameter with an error message in the query string.For example, assume I have a Doorkeeper-backed Identity provider at
identity-provider.com
. It has an application with a client ID of123
and a redirect-URI ofhttps://real-site.com/callback
.If I have previously authorized with
identity-provider.com
with theopenid profile
claims, and I issue a new authorization request with only theopenid
scope, andprompt=none
, an error is raised because the claims I'm requesting don't match the claims I previously authorized, butprompt=none
prevents me from reauthorizing.The error causes me to be redirected to whatever
redirect_uri
is included in the request, even if it's not a valid whitelisted redirect URI for the subject application.A sample request, assuming I'm already authorized (newlines added for clarity):
This request sends me to (again, newlines added for clarity):
At this point,
attacksite.com
can display a phishing form; given that I clicked on a link that started withidentity-provider.com
to start the authorization flow, it's possible that I can be tricked into entering my credentials into the attacking site. This is (I believe) an example of a Dangerous URL Redirect.The problem seems to be in how exceptions are rescued:
https://github.com/meagar/doorkeeper-openid_connect/blob/12685b19af46e0e1abcf6c74efe919268fec34f8/lib/doorkeeper/openid_connect/helpers/controller.rb#L22
This line extracts
redirect_uri
directly from the incoming parameters, and does nothing to validate it before redirecting the browser there on subsequent lines.I think the correct behaviour here is to render a 422 error, rather than redirect the browser, regardless of the
prompt=none
.The text was updated successfully, but these errors were encountered: