Fix CodeQL finding - regex - #3926
Merged
Merged
Conversation
Changes:
- server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaUrlUtils.java:115-134
- Replaced the unbounded userinfo group (.*:.*@)? with ([^:/@]{0,255}:[^/@]{0,255}@)? — character classes that exclude the surrounding delimiters, so the regex engine has only one way to
match each character (no quadratic backtracking) and is hard-bounded at 255 chars per part.
- Added a MAX_REDIRECT_URI_LENGTH = 2048 length cap in isValidRegisteredRedirectUrl so pathological inputs are rejected before the regex ever runs.
- server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaUrlUtilsTest.java:649-672
- New isValidRegisteredRedirectUrlRejectsOverlongInputQuickly test: feeds 100 000 a's, asserts false and that the call returns in under 1 second. This guards against future regressions
reintroducing polynomial behaviour.
Test results: 172 tests in UaaUrlUtilsTest pass, 92 tests across ClientAdminEndpointsValidatorTests and LegacyRedirectResolverTest pass — all existing redirect-URI semantics (including userinfo
edge cases like http://username:password@some.server.com, http://*:*@some.server.com, and the http://AAA@@attacker.com host-confusion rejections) are preserved.
The CodeQL alert at line 126 should now clear: the regex is no longer polynomial, and the input is length-bounded before evaluation.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR mitigates a CodeQL ReDoS finding in redirect URI validation by bounding redirect URI input length and tightening the userinfo portion of the registered redirect URI regex.
Changes:
- Adds a 8000-character maximum for registered redirect URI validation.
- Replaces the unbounded userinfo regex group with bounded character classes.
- Adds a regression test for overlong redirect URI rejection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaUrlUtils.java |
Adds redirect URI length limiting and updates the allowed redirect URI pattern. |
server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaUrlUtilsTest.java |
Adds a test for quick rejection of overlong redirect URI inputs. |
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
duanemay
approved these changes
Jun 1, 2026
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes:
Test results: 172 tests in UaaUrlUtilsTest pass, 92 tests across ClientAdminEndpointsValidatorTests and LegacyRedirectResolverTest pass — all existing redirect-URI semantics (including userinfo
edge cases like http://username:password@some.server.com, http://:@some.server.com, and the http://AAA@@attacker.com host-confusion rejections) are preserved.
The CodeQL alert at line 126 should now clear: the regex is no longer polynomial, and the input is length-bounded before evaluation.