Diff options
| author | Bryce Kwon <bryce@brycekwon.com> | |
|---|---|---|
| committer | Bryce Kwon <bryce@brycekwon.com> | |
| commit | ||
| parent | ||
| tree | ||
| download | ||
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.
Diffstat (limited to 'custom/extensions')
| -rw-r--r-- | custom/extensions/auth-file.lua | 10 | |
| -rw-r--r-- | custom/extensions/auth-inline.lua | 10 |
2 files changed, 12 insertions, 8 deletions
diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua index 6cbb8e8..1ad2eb8 100644 --- a/custom/extensions/auth-file.lua +++ b/custom/extensions/auth-file.lua @@ -327,11 +327,13 @@ function get_secret() return secret end --- A redirect target is unsafe if a browser would read it as another origin. --- The only such form cgit can be tricked into signing is a scheme-relative --- "//host" or "/\host". Everything else stays on this host. +-- A redirect target is safe only as a path on this host, which starts with +-- a single "/". A browser reads "//host" and "/\host" as another origin, and +-- a target that does not start with "/" can name a scheme. cgit signs the +-- page's own url, which starts with virtual-root, so logins need that to +-- be a path, as it is by default. function is_safe_redirect(url) - if type(url) ~= "string" then + if type(url) ~= "string" or url:sub(1, 1) ~= "/" then return false end local head = url:sub(1, 2) diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua index b97bb27..77084e2 100644 --- a/custom/extensions/auth-inline.lua +++ b/custom/extensions/auth-inline.lua @@ -290,11 +290,13 @@ function get_secret() return secret end --- A redirect target is unsafe if a browser would read it as another origin. --- The only such form cgit can be tricked into signing is a scheme-relative --- "//host" or "/\host". Everything else stays on this host. +-- A redirect target is safe only as a path on this host, which starts with +-- a single "/". A browser reads "//host" and "/\host" as another origin, and +-- a target that does not start with "/" can name a scheme. cgit signs the +-- page's own url, which starts with virtual-root, so logins need that to +-- be a path, as it is by default. function is_safe_redirect(url) - if type(url) ~= "string" then + if type(url) ~= "string" or url:sub(1, 1) ~= "/" then return false end local head = url:sub(1, 2) |