rusackas commented on PR #39646: URL: https://github.com/apache/superset/pull/39646#issuecomment-4434646671
Chiming in to echo some findings/status on this one. 1. The core concern raised by @hainenber is valid. Blocking private network addresses unconditionally breaks any Superset deployment where the OAuth2 provider is internal (e.g., enterprise SSO, self-hosted Keycloak, etc.). This is a real usability regression for many legitimate deployments. The ALLOWED_HOSTS escape hatch partially mitigates this, but operators would need to know to configure it. 2. DNS TOCTOU / rebinding risk. The bot reviewers correctly identified that even the second commit's use of getaddrinfo() doesn't fully close the race: the validation resolves DNS at configuration time, but requests.post() does its own resolution later. A DNS rebinding attack can still slip through. This is a hard problem to solve purely at the application layer without using a hardened HTTP client (like ssrf-guard, or a network-level control). 3. No tests. There are no unit tests covering the validation logic or the new config flags. 4. The fix scope is narrow. The PR title mentions multiple outbound HTTP requests in base.py and impala.py, but only base.py is modified. The Impala path remains unaddressed. Let us know if/how you want to carry this forward from here, and if you think there's a real security risk here, please follow the security submission process on the GitHub Issue template rather than publicly surfacing anything in an Issue or publishing exploits in a PR description. Just email [email protected], basically. Maybe as a half-measure, we can set `DATABASE_OAUTH2_TOKEN_URI_SSRF_VALIDATION` to `False` for now (opt-in rather than opt-out) until the UX impact is better understood, or require a more explicit allowlist model. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
