Diff options
| author | Bryce Kwon <bryce@brycekwon.com> | |
|---|---|---|
| committer | Bryce Kwon <bryce@brycekwon.com> | |
| commit | ||
| parent | ||
| tree | ||
| download | ||
Strip control characters from the location alone
set_cookie only ever writes an empty value or a token secure_value
built from url-encoded text, digits and hex, so stripping it changed
nothing.
| -rw-r--r-- | custom/extensions/auth-file.lua | 15 | |
| -rw-r--r-- | custom/extensions/auth-inline.lua | 15 | |
| -rw-r--r-- | tests/extensions/test-auth.lua | 2 |
3 files changed, 12 insertions, 20 deletions
diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua index 7d4d514..96c2074 100644 --- a/custom/extensions/auth-file.lua +++ b/custom/extensions/auth-file.lua @@ -391,12 +391,6 @@ function secure_value(field, value, expiration, binding) return payload .. "|" .. sign(payload, binding) end --- A control character in a header value would let that value split the --- response, so it is dropped rather than escaped. -function strip_controls(s) - return (string.gsub(s or "", "%c", "")) -end - -- Compare two strings in time that does not depend on how many leading bytes -- match, so a mismatch position is not revealed by timing. function constant_equals(a, b) @@ -412,7 +406,8 @@ function constant_equals(a, b) end -- An empty value is how a login is cleared, since Max-Age=0 tells the browser --- to drop the cookie it already holds. +-- to drop the cookie it already holds. Any other value is a token secure_value +-- built, which holds no control character to split the response with. function set_cookie(cookie, value) local attributes = "; HttpOnly; SameSite=Lax; Path=" .. cookie_path if not cookie_insecure then @@ -423,13 +418,15 @@ function set_cookie(cookie, value) elseif session_seconds > 0 then attributes = attributes .. "; Max-Age=" .. tostring(session_seconds) end - html("Set-Cookie: " .. cookie .. "=" .. strip_controls(value) .. attributes .. "\n") + html("Set-Cookie: " .. cookie .. "=" .. value .. attributes .. "\n") end +-- A control character in the location would let it split the response, so +-- it is dropped rather than escaped. function redirect_to(url) html("Status: 302 Found\n") html("Cache-Control: no-cache, no-store\n") - html("Location: " .. strip_controls(url) .. "\n") + html("Location: " .. url:gsub("%c", "") .. "\n") end -- A failed login lands back on the page it came from with this flag, which diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua index c9aa9f1..6c0ad3b 100644 --- a/custom/extensions/auth-inline.lua +++ b/custom/extensions/auth-inline.lua @@ -352,12 +352,6 @@ function secure_value(field, value, expiration, binding) return payload .. "|" .. sign(payload, binding) end --- A control character in a header value would let that value split the --- response, so it is dropped rather than escaped. -function strip_controls(s) - return (string.gsub(s or "", "%c", "")) -end - -- Compare two strings in time that does not depend on how many leading bytes -- match, so a mismatch position is not revealed by timing. function constant_equals(a, b) @@ -373,7 +367,8 @@ function constant_equals(a, b) end -- An empty value is how a login is cleared, since Max-Age=0 tells the browser --- to drop the cookie it already holds. +-- to drop the cookie it already holds. Any other value is a token secure_value +-- built, which holds no control character to split the response with. function set_cookie(cookie, value) local attributes = "; HttpOnly; SameSite=Lax; Path=" .. cookie_path if not cookie_insecure then @@ -384,13 +379,15 @@ function set_cookie(cookie, value) elseif session_seconds > 0 then attributes = attributes .. "; Max-Age=" .. tostring(session_seconds) end - html("Set-Cookie: " .. cookie .. "=" .. strip_controls(value) .. attributes .. "\n") + html("Set-Cookie: " .. cookie .. "=" .. value .. attributes .. "\n") end +-- A control character in the location would let it split the response, so +-- it is dropped rather than escaped. function redirect_to(url) html("Status: 302 Found\n") html("Cache-Control: no-cache, no-store\n") - html("Location: " .. strip_controls(url) .. "\n") + html("Location: " .. url:gsub("%c", "") .. "\n") end -- A failed login lands back on the page it came from with this flag, which diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua index 48c2add..07c7341 100644 --- a/tests/extensions/test-auth.lua +++ b/tests/extensions/test-auth.lua @@ -161,8 +161,6 @@ 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("a missing target is not", not is_safe_redirect(nil)) -h.equals("control characters are stripped from header values", strip_controls("a\r\nb"), "ab") - -- Signing and verification. local now = os.time() |