diff --git a/lib/hexdocs/plug.ex b/lib/hexdocs/plug.ex index 10d8a49..dd1986b 100644 --- a/lib/hexdocs/plug.ex +++ b/lib/hexdocs/plug.ex @@ -97,7 +97,7 @@ defmodule Hexdocs.Plug do conn |> put_session("oauth_code_verifier", code_verifier) |> put_session("oauth_state", state) - |> put_session("oauth_return_path", conn.request_path) + |> put_session("oauth_return_path", safe_return_path(conn.request_path)) |> redirect(url) end @@ -362,6 +362,16 @@ defmodule Hexdocs.Plug do end) end + defp safe_return_path("/" <> rest = path) do + if String.starts_with?(rest, "/") do + "/" + else + path + end + end + + defp safe_return_path(_), do: "/" + defp redirect(conn, url) do html = Plug.HTML.html_escape(url) body = "You are being redirected." diff --git a/test/hexdocs/plug_test.exs b/test/hexdocs/plug_test.exs index b04cca0..2a93652 100644 --- a/test/hexdocs/plug_test.exs +++ b/test/hexdocs/plug_test.exs @@ -38,6 +38,24 @@ defmodule Hexdocs.PlugTest do assert get_session(conn, "oauth_return_path") == "/foo" end + test "sanitize protocol-relative return path to prevent open redirect" do + conn = conn(:get, "http://plugtest.localhost:5002//evil.com") |> call() + assert conn.status == 302 + assert get_session(conn, "oauth_return_path") == "/" + end + + test "sanitize protocol-relative return path with extra slashes" do + conn = conn(:get, "http://plugtest.localhost:5002///evil.com/foo") |> call() + assert conn.status == 302 + assert get_session(conn, "oauth_return_path") == "/" + end + + test "preserve valid return path" do + conn = conn(:get, "http://plugtest.localhost:5002/some/path") |> call() + assert conn.status == 302 + assert get_session(conn, "oauth_return_path") == "/some/path" + end + test "OAuth callback with invalid state returns error" do conn = conn(:get, "http://plugtest.localhost:5002/oauth/callback?code=abc&state=wrong")