Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deep validation of redirect URI #3579

Open
2 tasks done
nabokihms opened this issue Jun 13, 2024 · 1 comment
Open
2 tasks done

Deep validation of redirect URI #3579

nabokihms opened this issue Jun 13, 2024 · 1 comment

Comments

@nabokihms
Copy link
Member

nabokihms commented Jun 13, 2024

Preflight Checklist

  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Problem Description

https://security.lauritz-holtmann.de/post/sso-security-redirect-uri/

Proposed Solution

Validate schema and query parameters. Do not allow the following:

  • Schema: data, javascript, vbscript
  • Query params: code, state, response

Alternatives Considered

No response

Additional Information

No response

@abhisek
Copy link
Contributor

abhisek commented Jul 4, 2024

@nabokihms I was looking at this issue and it seems to me that the attack will require a malicious client registration, either as static client or through the gRPC API.

From documentation:

The Dex API does not provide any authentication or authorization beyond TLS client auth.

Since there is no AuthZ at the API level, access to the gRPC API can be practically considered as administrative access to dex. So we cannot consider the case where a malicious client exploits this issue to gain additional privilege. Also dex doesn't really have any session so there isn't anything to protect even if malicious client side code is executed.

To me it doesn't seem like a risk worth fixing at this point. If we need to harden the redirect URI validation then we should probably consider an allow list approach because I see there are possibility of attacks using the ws:// scheme as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants