Skip to content

Commit d621120

Browse files
B4nanclaude
andauthored
fix: exclude backslashes from trailing-slash rewrite to prevent open redirect (#2359)
## Summary - Previous `if`-based checks on `$request_uri` and `$uri` didn't work in production despite passing CI - This changes the trailing-slash location regex from `^(.+)/$` to `^([^\\\\]+)/$` so it simply never matches URIs containing backslashes - No redirect fires → no `\` in the Location header → no open redirect - Locally verified against real nginx: attack vectors get 404, normal redirects still work 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 5093645 commit d621120

2 files changed

Lines changed: 27 additions & 16 deletions

File tree

.github/workflows/test.yaml

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,31 @@ jobs:
8585
[ "$actual" = "$expected" ] || (echo "❌ Expected HTTP $expected but got $actual for $url" && exit 1)
8686
}
8787
88+
function assert_no_redirect() {
89+
url=$1
90+
shift
91+
extra_args=("$@")
92+
response=$(curl -s -D - -o /dev/null -w "\n%{http_code}" "${extra_args[@]}" "$url" 2>/dev/null)
93+
status=$(echo "$response" | tail -1)
94+
location=$(echo "$response" | grep -i "^location:" | tr -d '\r' || true)
95+
echo "→ $url → HTTP $status ${location:+(${location})}"
96+
if [ "$status" = "301" ] || [ "$status" = "302" ]; then
97+
echo "❌ Got redirect for $url: $location" && exit 1
98+
fi
99+
}
100+
88101
echo "🧪 Checking open redirect protection..."
89-
# nginx decodes %5C to \ in $uri, so the check catches both forms.
90-
assert_status "http://localhost:8080///%5Cevil.com/" "400"
91-
assert_status "http://localhost:8080/%5Cevil.com/" "400"
92-
assert_status "http://localhost:8080///%5cevil.com/" "400"
102+
# Backslash URLs must not produce redirects (the redirect Location
103+
# would contain \, which browsers normalize to /, creating
104+
# protocol-relative URLs like //evil.com that redirect externally).
105+
assert_no_redirect "http://localhost:8080///%5Cevil.com/"
106+
assert_no_redirect "http://localhost:8080/%5Cevil.com/"
107+
assert_no_redirect "http://localhost:8080///%5cevil.com/"
93108
# Literal backslash (simulates CDN pre-decoding %5C before forwarding)
94-
assert_status "http://localhost:8080" "400" --request-target '/\evil.com/'
95-
assert_status "http://localhost:8080" "400" --request-target '///\evil.com/'
109+
assert_no_redirect "http://localhost:8080" --request-target '/\evil.com/'
110+
assert_no_redirect "http://localhost:8080" --request-target '///\evil.com/'
111+
# Normal trailing-slash redirect must still work
112+
assert_status "http://localhost:8080/platform/proxy/usage/" "302"
96113
97114
echo "🧪 Checking Nginx responses... (apify-docs)"
98115
assert_header "http://localhost:8080/" "Content-Type" "text/html"

nginx.conf

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,6 @@ 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 in URL
25-
# The trailing-slash rewrite puts \ in the Location header, which browsers
26-
# normalize to /, creating protocol-relative URLs (e.g. /\evil.com → //evil.com).
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 ~ "\\\\") {
30-
return 400;
31-
}
32-
3324
location = / {
3425
if ($serve_markdown) {
3526
rewrite ^ /llms.txt last;
@@ -45,7 +36,10 @@ server {
4536

4637
# remove trailing slashes from all URLs (except root /)
4738
# exact match locations (e.g., location = /sdk/js/) take priority over this regex
48-
location ~ ^(.+)/$ {
39+
# [^\\\\] excludes backslashes to prevent open redirect: nginx decodes %5C to \ in $uri,
40+
# and \ in the Location header gets normalized to / by browsers, turning /\evil.com
41+
# into the protocol-relative URL //evil.com which redirects to evil.com.
42+
location ~ ^([^\\\\]+)/$ {
4943
rewrite ^(.+)/$ $1$is_args$args? redirect;
5044
}
5145

0 commit comments

Comments
 (0)