Skip to content

Commit 1dbbdda

Browse files
committed
fix: percent-encoded path bypass of ManagementInterceptor loopback gate
- 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
1 parent 530356f commit 1dbbdda

2 files changed

Lines changed: 37 additions & 3 deletions

File tree

components/management/src/main/kotlin/org/cloudfoundry/credhub/ManagementInterceptor.kt

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import org.cloudfoundry.credhub.exceptions.ReadOnlyException
77
import org.springframework.stereotype.Component
88
import org.springframework.web.bind.annotation.RequestMethod
99
import org.springframework.web.servlet.AsyncHandlerInterceptor
10+
import org.springframework.web.util.UriUtils
11+
import java.nio.charset.StandardCharsets
1012

1113
@Component
1214
class ManagementInterceptor(
@@ -17,14 +19,28 @@ class ManagementInterceptor(
1719
response: HttpServletResponse,
1820
handler: Any,
1921
): Boolean {
20-
if (request.requestURI == MANAGEMENT_API && request.remoteAddr != request.localAddr) {
22+
// Decode the raw requestURI so that percent-encoded paths like /%6Danagement are
23+
// normalised to /management before comparison — mirroring exactly what Tomcat's
24+
// CoyoteAdapter does via UDecoder.convert() when populating servletPath, and
25+
// consistent with how Spring Security's PathPatternRequestMatcher and Spring MVC's
26+
// handler mapping both resolve the effective path from the decoded form.
27+
// Using requestURI (rather than servletPath) also works in MockMvc, where the
28+
// servlet container never populates servletPath and it stays as "".
29+
val contextPath = request.contextPath ?: ""
30+
val decodedPath =
31+
UriUtils.decode(
32+
request.requestURI.removePrefix(contextPath),
33+
StandardCharsets.UTF_8,
34+
)
35+
36+
if (decodedPath == MANAGEMENT_API && request.remoteAddr != request.localAddr) {
2137
throw InvalidRemoteAddressException()
2238
}
2339

2440
if (managementRegistry.readOnlyMode &&
2541
!request.method.equals(RequestMethod.GET.toString(), ignoreCase = true) &&
26-
request.requestURI != MANAGEMENT_API &&
27-
request.requestURI != INTERPOLATE_API
42+
decodedPath != MANAGEMENT_API &&
43+
decodedPath != INTERPOLATE_API
2844
) {
2945
throw ReadOnlyException()
3046
}

components/management/src/test/kotlin/org/cloudfoundry/credhub/interceptors/ManagementInterceptorTest.kt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,22 @@ class ManagementInterceptorTest {
9494
request!!.method = "POST"
9595
subject!!.preHandle(request!!, response!!, Any())
9696
}
97+
98+
@Test
99+
fun preHandle_throwsExceptionForEncodedManagementPathFromRemoteAddress() {
100+
request!!.remoteAddr = "10.0.0.1"
101+
request!!.localAddr = "127.0.0.1"
102+
request!!.requestURI = "/%6Danagement"
103+
assertThrows<InvalidRemoteAddressException> {
104+
subject!!.preHandle(request!!, response!!, Any())
105+
}
106+
}
107+
108+
@Test
109+
fun preHandle_encodedInterpolatePathIsExemptFromReadOnlyMode() {
110+
managementRegistry!!.readOnlyMode = true
111+
request!!.requestURI = "/%69nterpolate"
112+
request!!.method = "POST"
113+
subject!!.preHandle(request!!, response!!, Any())
114+
}
97115
}

0 commit comments

Comments
 (0)