fix: match request.uri on the forwarded path (absolute-form ACL bypass) by winklemad · Pull Request #614 · diennea/carapaceproxy

winklemad · 2026-07-12T07:36:06Z

No linked issue — found by reviewing the request-matching vs. forwarding paths.

Summary (security)

A route/ACL written against request.uri can be bypassed by sending the request in absolute-form, because the matcher and the backend evaluate two different views of the request path.

The matcher property request.uri returns the raw request.uri() — for an absolute-form request target this is http://host/path (ProxyRequest.getProperty, PROPERTY_URI).
The value actually forwarded to the backend is the normalized path from getUri(), which strips the scheme://authority prefix — i.e. just /path (ProxyRequest.getUri(), used at ProxyRequestsManager.forward()).
RegexpRequestMatcher uses full-match semantics (Pattern.matcher(...).matches()).

So a path-anchored deny rule does not match the absolute form and is skipped:

# operator intends to block /admin
route.0.match  = request.uri ~ "/admin/.*"     # -> deny (403)
route.1.match  = all                            # -> proxy to backend

GET http://victim.example/admin/secret HTTP/1.1     # absolute-form, valid HTTP/1.1
Host: victim.example

The matcher sees http://victim.example/admin/secret; the regex /admin/.* under .matches() must match the whole string (which starts with http), so route 0 does not match → the deny is skipped.
The request falls through to route 1 (all) → PROXY. getUri() strips the scheme prefix, so the backend receives GET /admin/secret — the resource the ACL was meant to block.

Impact: CWE-436 (interpretation conflict between proxy and backend) / CWE-863 (incorrect authorization). Any deployment that uses a path-anchored request.uri rule as an access-control boundary is affected.

Fix

Make the request.uri matcher property return getUri() — the same normalized path that is forwarded — so the matcher and the backend can never disagree. This is a no-op for normal origin-form requests (there getUri() equals request.uri()); it only changes the absolute-form case.

Testing

Added RequestMatcherTest.testAbsoluteFormRequestTargetMatchesPathRule, which asserts that an absolute-form request target still matches a path-anchored rule (request.uri ~ "/admin/.*"). It fails before the change and passes after. The existing RequestMatcherTest still passes (its request mock now also stubs fullPath(), which a real reactor-netty request always provides). Verified on a full Java 21 Maven build; no new checkstyle violations.

The RequestMatcher `request.uri` property returned the raw `request.uri()`, while the request is forwarded to the backend as the normalized path from `getUri()` (which strips a `scheme://authority` prefix). For an absolute-form request target -- which is valid HTTP/1.1 -- the two disagree: the matcher sees `http://host/path` while the backend receives just `/path`. Because `RegexpRequestMatcher` uses full-match semantics, a path-anchored route or ACL such as `request.uri ~ "/admin/.*"` does not match the absolute form and is skipped; the request then falls through to a proxy route and is forwarded to the backend as `/admin/...`, bypassing the ACL. Match on `getUri()` so the matcher and the backend evaluate the same path. This is a no-op for normal origin-form requests (`getUri()` == `request.uri()`) and only changes the absolute-form case. Signed-off-by: winklemad <[email protected]>

coderabbitai · 2026-07-12T07:36:25Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1bb79d75-63d5-48cc-94fe-6d043a3b8aea

📥 Commits

Reviewing files that changed from the base of the PR and between a680487 and 73cc866.

📒 Files selected for processing (1)

carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java

🚧 Files skipped from review as they are similar to previous changes (1)

carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java

📝 Walkthrough

Walkthrough

ProxyRequest.getProperty now exposes the normalized URI from getUri() for PROPERTY_URI instead of Netty’s raw request target. Request matcher tests now mock fullPath() and verify that absolute-form request targets match path-based rules using the forwarded backend path.

Poem

I’m a rabbit with a path to share,
No raw URI can hop elsewhere.
/admin/secret follows the trail,
While matching rules will never fail.
Normalize, test, and cheer—
The backend path is clear! 🐇

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java (1)
189-198: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Reuse the fullPath local variable instead of calling request.fullPath() again.

Line 198 calls request.fullPath() a second time (already computed at line 189 into fullPath). This is now on the request-matching hot path since getProperty(PROPERTY_URI) calls getUri(). Reusing the local variable avoids a redundant call and keeps the logic DRY.
♻️ Proposed refactor
         } else {
-            int pos = uri.indexOf(request.fullPath());
+            int pos = uri.indexOf(fullPath);
             if (pos > 0) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java`
around lines 189 - 198, Update the URI construction logic in ProxyRequest.getUri
to reuse the existing fullPath local variable instead of calling
request.fullPath() again in the non-blank branch. Preserve the current
path-matching behavior while eliminating the redundant request.fullPath()
invocation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java`:
- Around line 189-198: Update the URI construction logic in ProxyRequest.getUri
to reuse the existing fullPath local variable instead of calling
request.fullPath() again in the non-blank branch. Preserve the current
path-matching behavior while eliminating the redundant request.fullPath()
invocation.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 97bdd7e8-92b3-422f-8790-fd7a149e7f21

📥 Commits

Reviewing files that changed from the base of the PR and between f625509 and a680487.

📒 Files selected for processing (2)

carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java
carapace-server/src/test/java/org/carapaceproxy/server/mapper/requestmatcher/RequestMatcherTest.java

…) call Signed-off-by: winklemad <[email protected]>

NiccoMlt

Looks good to me, thank you very much for the contribution and for the thorough write-up!

coderabbitai Bot reviewed Jul 12, 2026

View reviewed changes

reuse fullPath local in getUri to avoid a redundant request.fullPath(…

73cc866

…) call Signed-off-by: winklemad <[email protected]>

NiccoMlt self-assigned this Jul 15, 2026

NiccoMlt approved these changes Jul 15, 2026

View reviewed changes

NiccoMlt merged commit c593835 into diennea:master Jul 15, 2026
2 checks passed

Navigation Menu

Repository navigation

fix: match request.uri on the forwarded path (absolute-form ACL bypass) - #614

fix: match request.uri on the forwarded path (absolute-form ACL bypass)#614
NiccoMlt merged 2 commits into
diennea:masterfrom
winklemad:fix-absolute-form-acl-bypass

winklemad commented Jul 12, 2026

Uh oh!

coderabbitai Bot commented Jul 12, 2026 •

edited

Loading

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Uh oh!

NiccoMlt left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

winklemad commented Jul 12, 2026

Summary (security)

Fix

Testing

Uh oh!

coderabbitai Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

NiccoMlt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jul 12, 2026 •

edited

Loading