From cb9186bd0d11bd5a5dd6c276cb7071422850bd54 Mon Sep 17 00:00:00 2001 From: Bryce Kwon Date: Sat, 7 Nov 2026 14:14:55 -1000 Subject: Accept only a local path as the way back in is_safe_redirect looked at the first two characters alone, so it accepted https://evil.example/ and javascript:x. Neither could be signed today only because cgit always signs a url that starts with virtual-root, and nothing tested that. A target now has to start with a single slash, which needs virtual-root to be a path for logins to work, as it is by default. --- tests/extensions/test-auth.lua | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'tests/extensions') diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua index 3470741..86f8f89 100644 --- a/tests/extensions/test-auth.lua +++ b/tests/extensions/test-auth.lua @@ -256,8 +256,14 @@ h.check("differing lengths do not", not constant_equals("a", "ab")) h.check("non strings do not", not constant_equals(nil, "a")) h.check("a local path is a safe redirect", is_safe_redirect("/x")) +h.check("and so is the root", is_safe_redirect("/")) h.check("a scheme relative target is not", not is_safe_redirect("//evil")) h.check("a backslash variant is not", not is_safe_redirect("/\\evil")) +h.check("nor one of backslashes alone", not is_safe_redirect("\\\\evil")) +h.check("nor an absolute url", not is_safe_redirect("https://evil.example/")) +h.check("nor any other scheme", not is_safe_redirect("javascript:x")) +h.check("nor a relative path", not is_safe_redirect("x/y")) +h.check("nor an empty target", not is_safe_redirect("")) h.check("a missing target is not", not is_safe_redirect(nil)) -- Signing and verification. @@ -464,6 +470,8 @@ h.excludes("over https it says nothing of the kind", out, "only works over https out = run_action("body", { url = "//evil.example/x" }) h.equals("an unsafe destination is swapped for the login page", way_back_in(out), "/?p=login") h.equals("and is not taken for a failed login", out:find("login-error", 1, true), nil) +out = run_action("body", { url = "https://evil.example/x" }) +h.equals("and so is one with a scheme", way_back_in(out), "/?p=login") out = run_action("body", { url = "/repo/log/?q=x&login=failed" }) h.contains("a failed login says so", out, @@ -571,6 +579,8 @@ h.contains("a post without the signed token is not served", out, "Status: 404") out = login("Alice", password, secure_value("redirect", "//evil.example/", 0)) h.contains("even a signed unsafe destination is not followed", out, "Status: 404") +out = login("Alice", password, secure_value("redirect", "https://evil.example/", 0)) +h.contains("nor a signed one with a scheme", out, "Status: 404") -- Over plain http the cookie cannot be Secure, and so loses the __Host- name. load_script({ { "local cookie_insecure = false", "local cookie_insecure = true" } }) -- cgit 2.12.1