Skip to content

Commit 5093645

Browse files
B4nanclaude
andauthored
fix: use $uri for backslash open redirect check (#2357)
## Summary - The previous `$request_uri` check doesn't work in production because CloudFront decodes `%5C` to literal `\` before forwarding to nginx, so `$request_uri` never contains `%5C` - Switches to `$uri` which nginx always decodes internally (`%5C` → `\`), regardless of CDN behavior - Verified with `curl` against the live site: `/%5Ctest` returns 404 (not 400), confirming CloudFront strips the encoding before it reaches nginx Fixes apify/apify-core#26551 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ba47ab9 commit 5093645

2 files changed

Lines changed: 7 additions & 8 deletions

File tree

.github/workflows/test.yaml

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,13 @@ jobs:
8686
}
8787
8888
echo "🧪 Checking open redirect protection..."
89-
# Encoded backslash (%5C) - as sent by clients directly
89+
# nginx decodes %5C to \ in $uri, so the check catches both forms.
9090
assert_status "http://localhost:8080///%5Cevil.com/" "400"
9191
assert_status "http://localhost:8080/%5Cevil.com/" "400"
9292
assert_status "http://localhost:8080///%5cevil.com/" "400"
93-
# Literal backslash - as forwarded by CDNs that decode %5C before proxying
93+
# Literal backslash (simulates CDN pre-decoding %5C before forwarding)
9494
assert_status "http://localhost:8080" "400" --request-target '/\evil.com/'
9595
assert_status "http://localhost:8080" "400" --request-target '///\evil.com/'
96-
# Multiple leading slashes (no backslash) - also a redirect vector
97-
assert_status "http://localhost:8080" "400" --request-target '//evil.com/'
9896
9997
echo "🧪 Checking Nginx responses... (apify-docs)"
10098
assert_header "http://localhost:8080/" "Content-Type" "text/html"

nginx.conf

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,12 @@ server {
2121
set $backend "https://apify.github.io/apify-docs";
2222
resolver 1.1.1.1 8.8.8.8 valid=30s ipv6=off;
2323

24-
# Prevent open redirect via backslash or multiple leading slashes in URL
25-
# CDNs like CloudFront may decode %5C to literal \ before forwarding to nginx.
26-
# The trailing-slash rewrite then puts \ in the Location header, which browsers
24+
# Prevent open redirect via backslash in URL
25+
# The trailing-slash rewrite puts \ in the Location header, which browsers
2726
# normalize to /, creating protocol-relative URLs (e.g. /\evil.com → //evil.com).
28-
if ($request_uri ~ "\\\\|%5[cC]|^//") {
27+
# Using $uri (not $request_uri) because nginx always decodes %5C → \ in $uri,
28+
# regardless of whether CloudFront or other CDNs pre-decode the request.
29+
if ($uri ~ "\\\\") {
2930
return 400;
3031
}
3132

0 commit comments

Comments
 (0)