Skip to content

Commit cc2e7ca

Browse files
committed
Fix open redirect vulnerability in OAuth return path
Validate that the OAuth return path stored in the session is a safe relative path. Protocol-relative URLs (e.g. //evil.com) are rejected and replaced with "/" to prevent redirecting users to external domains after the OAuth flow completes.
1 parent 6d12b51 commit cc2e7ca

2 files changed

Lines changed: 29 additions & 1 deletion

File tree

lib/hexdocs/plug.ex

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ defmodule Hexdocs.Plug do
9797
conn
9898
|> put_session("oauth_code_verifier", code_verifier)
9999
|> put_session("oauth_state", state)
100-
|> put_session("oauth_return_path", conn.request_path)
100+
|> put_session("oauth_return_path", safe_return_path(conn.request_path))
101101
|> redirect(url)
102102
end
103103

@@ -362,6 +362,16 @@ defmodule Hexdocs.Plug do
362362
end)
363363
end
364364

365+
defp safe_return_path("/" <> rest = path) do
366+
if String.starts_with?(rest, "/") do
367+
"/"
368+
else
369+
path
370+
end
371+
end
372+
373+
defp safe_return_path(_), do: "/"
374+
365375
defp redirect(conn, url) do
366376
html = Plug.HTML.html_escape(url)
367377
body = "<html><body>You are being <a href=\"#{html}\">redirected</a>.</body></html>"

test/hexdocs/plug_test.exs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,24 @@ defmodule Hexdocs.PlugTest do
3838
assert get_session(conn, "oauth_return_path") == "/foo"
3939
end
4040

41+
test "sanitize protocol-relative return path to prevent open redirect" do
42+
conn = conn(:get, "http://plugtest.localhost:5002//evil.com") |> call()
43+
assert conn.status == 302
44+
assert get_session(conn, "oauth_return_path") == "/"
45+
end
46+
47+
test "sanitize protocol-relative return path with extra slashes" do
48+
conn = conn(:get, "http://plugtest.localhost:5002///evil.com/foo") |> call()
49+
assert conn.status == 302
50+
assert get_session(conn, "oauth_return_path") == "/"
51+
end
52+
53+
test "preserve valid return path" do
54+
conn = conn(:get, "http://plugtest.localhost:5002/some/path") |> call()
55+
assert conn.status == 302
56+
assert get_session(conn, "oauth_return_path") == "/some/path"
57+
end
58+
4159
test "OAuth callback with invalid state returns error" do
4260
conn =
4361
conn(:get, "http://plugtest.localhost:5002/oauth/callback?code=abc&state=wrong")

0 commit comments

Comments
 (0)