Skip to content

Commit 3573848

Browse files
committed
Fix insecure path handling in redirections.
1 parent e8c13eb commit 3573848

3 files changed

Lines changed: 23 additions & 4 deletions

File tree

lib/utopia/redirection.rb

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,20 +89,26 @@ def cache_control
8989
"max-age=#{self.max_age}"
9090
end
9191

92-
def headers(location)
93-
{HTTP::LOCATION => location, HTTP::CACHE_CONTROL => self.cache_control}
92+
def make_headers(location)
93+
{
94+
HTTP::LOCATION => location,
95+
HTTP::CACHE_CONTROL => self.cache_control
96+
}
9497
end
9598

9699
def redirect(location)
97-
return [self.status, self.headers(location), []]
100+
return [self.status, self.make_headers(location), []]
98101
end
99102

100103
def [] path
101104
false
102105
end
103106

104107
def call(env)
105-
path = env[Rack::PATH_INFO]
108+
# Normalize the path to remove redundant slashes, `.` and `..` segments.
109+
# This prevents protocol-relative redirect URLs (e.g. //evil.com/index)
110+
# from being generated when PATH_INFO contains a double leading slash.
111+
path = Path.create(env[Rack::PATH_INFO]).simplify.to_s
106112

107113
if redirection = self[path]
108114
return redirection

releases.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Releases
22

3+
## Unreleasd
4+
5+
- **Security** Fix handling of redirects that start with `//` to prevent open redirect vulnerabilities.
6+
37
## v2.31.0
48

59
- Add agent context.

test/utopia/redirection.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@
1717
expect(last_response.headers["cache-control"]).to be(:include?, "max-age=86400")
1818
end
1919

20+
it "should not allow open redirect via protocol-relative URL" do
21+
get "//evil.com/"
22+
23+
# Must not redirect to //evil.com/index (external host)
24+
if last_response.status == 307
25+
expect(last_response.headers["location"]).not.to start_with("//")
26+
end
27+
end
28+
2029
it "should be permanently moved" do
2130
get "/a"
2231

0 commit comments

Comments
 (0)