Skip to content

fix: percent-encoded path bypass of ManagementInterceptor loopback gate#1154

Merged
hsinn0 merged 1 commit into
mainfrom
TNZGOV-4101
Jun 25, 2026
Merged

fix: percent-encoded path bypass of ManagementInterceptor loopback gate#1154
hsinn0 merged 1 commit into
mainfrom
TNZGOV-4101

Conversation

@hsinn0

@hsinn0 hsinn0 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor
  • A request to /%6Danagement (or any other percent-encoded equivalent) has requestURI = "/%6Danagement", which is not string-equal to "/management", so the loopback check was silently skipped
  • Spring Security's PathPatternRequestMatcher and Tomcat's servlet mapping both operate on the decoded path, so the request was still permitted and dispatched to ManagementController.updateManagementRegistry(), allowing an unauthenticated remote caller to toggle CredHub's read-only mode
  • Fix: decode requestURI with UriUtils.decode() (stripping context path first) before any comparison, mirroring what Tomcat's CoyoteAdapter.UDecoder does when producing servletPath; this works in both production (Tomcat decodes) and MockMvc tests (requestURI is always set, servletPath is not)
  • Apply the same decoded path variable to the read-only mode exemption checks for /management and /interpolate so encoded variants of those paths are also correctly handled

ai-assisted=yes
TNZGOV-4101

- A request to /%6Danagement (or any other percent-encoded equivalent) has
  requestURI = "/%6Danagement", which is not string-equal to "/management",
  so the loopback check was silently skipped
- Spring Security's PathPatternRequestMatcher and Tomcat's servlet mapping
  both operate on the decoded path, so the request was still permitted and
  dispatched to ManagementController.updateManagementRegistry(), allowing an
  unauthenticated remote caller to toggle CredHub's read-only mode
- Fix: decode requestURI with UriUtils.decode() (stripping context path first)
  before any comparison, mirroring what Tomcat's CoyoteAdapter.UDecoder does
  when producing servletPath; this works in both production (Tomcat decodes)
  and MockMvc tests (requestURI is always set, servletPath is not)
- Apply the same decoded path variable to the read-only mode exemption checks
  for /management and /interpolate so encoded variants of those paths are also
  correctly handled

ai-assisted=yes
TNZGOV-4101

@dlinsley dlinsley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a respective test!

@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Foundational Infrastructure Working Group Jun 25, 2026
@hsinn0 hsinn0 merged commit 1dbbdda into main Jun 25, 2026
2 checks passed
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Jun 25, 2026
@hsinn0 hsinn0 deleted the TNZGOV-4101 branch June 25, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants