From 6052c85118eda40b9f55e87718729748f9484652 Mon Sep 17 00:00:00 2001
From: Bryce Kwon <bryce@brycekwon.com>
Date: Sat, 31 Oct 2026 13:27:04 -0700
Subject: Take a signed cookie apart with one match

The loop let a valid cookie through with a field appended, a doubled
separator or a zero in front of its expiry. The payload now carries the
expiry as written, so only the spelling secure_value issues verifies.
---
 custom/extensions/auth-file.lua   | 46 +++++----------------------------------
 custom/extensions/auth-inline.lua | 46 +++++----------------------------------
 tests/extensions/test-auth.lua    |  1 +
 3 files changed, 11 insertions(+), 82 deletions(-)

diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua
index a03e1ea..f11954a 100644
--- a/custom/extensions/auth-file.lua
+++ b/custom/extensions/auth-file.lua
@@ -350,46 +350,9 @@ end
 -- or to nil to turn it down, which is how a session meets its account's
 -- current hash.
 function validate_value(expected_field, cookie, bind)
-	local i = 0
-	local value = ""
-	local field = ""
-	local expiration = 0
-	local salt = ""
-	local signature = ""
-
-	if cookie == nil or cookie:len() < 3 or cookie:sub(1, 1) == "|" then
-		return nil
-	end
-
-	for component in string.gmatch(cookie, "[^|]+") do
-		if i == 0 then
-			field = component
-		elseif i == 1 then
-			value = component
-		elseif i == 2 then
-			-- The expiration must be a plain integer, since
-			-- tonumber and tostring of "1e9" or "100.0" differ
-			-- across Lua versions and the signed bytes have to
-			-- come back byte for byte.
-			if not string.match(component, "^%d+$") then
-				return nil
-			end
-			expiration = tonumber(component)
-		elseif i == 3 then
-			salt = component
-		elseif i == 4 then
-			signature = component
-		else
-			break
-		end
-		i = i + 1
-	end
-
-	if signature == nil or signature:len() == 0 then
-		return nil
-	end
-
-	if url_decode(field) ~= expected_field then
+	local field, value, expiration, salt, signature =
+		string.match(cookie or "", "^([^|]+)|([^|]+)|(%d+)|([^|]+)|([^|]+)$")
+	if signature == nil or url_decode(field) ~= expected_field then
 		return nil
 	end
 
@@ -408,12 +371,13 @@ function validate_value(expected_field, cookie, bind)
 		end
 	end
 
-	local payload = field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt
+	local payload = field .. "|" .. value .. "|" .. expiration .. "|" .. salt
 	if not constant_equals(signature, sign(payload, binding)) then
 		return nil
 	end
 
 	-- An expiration of 0 never expires and is used for the redirect token.
+	expiration = tonumber(expiration)
 	if expiration ~= 0 and expiration <= os.time() then
 		return nil
 	end
diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua
index c4b87f6..7243e55 100644
--- a/custom/extensions/auth-inline.lua
+++ b/custom/extensions/auth-inline.lua
@@ -311,46 +311,9 @@ end
 -- or to nil to turn it down, which is how a session meets its account's
 -- current hash.
 function validate_value(expected_field, cookie, bind)
-	local i = 0
-	local value = ""
-	local field = ""
-	local expiration = 0
-	local salt = ""
-	local signature = ""
-
-	if cookie == nil or cookie:len() < 3 or cookie:sub(1, 1) == "|" then
-		return nil
-	end
-
-	for component in string.gmatch(cookie, "[^|]+") do
-		if i == 0 then
-			field = component
-		elseif i == 1 then
-			value = component
-		elseif i == 2 then
-			-- The expiration must be a plain integer, since
-			-- tonumber and tostring of "1e9" or "100.0" differ
-			-- across Lua versions and the signed bytes have to
-			-- come back byte for byte.
-			if not string.match(component, "^%d+$") then
-				return nil
-			end
-			expiration = tonumber(component)
-		elseif i == 3 then
-			salt = component
-		elseif i == 4 then
-			signature = component
-		else
-			break
-		end
-		i = i + 1
-	end
-
-	if signature == nil or signature:len() == 0 then
-		return nil
-	end
-
-	if url_decode(field) ~= expected_field then
+	local field, value, expiration, salt, signature =
+		string.match(cookie or "", "^([^|]+)|([^|]+)|(%d+)|([^|]+)|([^|]+)$")
+	if signature == nil or url_decode(field) ~= expected_field then
 		return nil
 	end
 
@@ -369,12 +332,13 @@ function validate_value(expected_field, cookie, bind)
 		end
 	end
 
-	local payload = field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt
+	local payload = field .. "|" .. value .. "|" .. expiration .. "|" .. salt
 	if not constant_equals(signature, sign(payload, binding)) then
 		return nil
 	end
 
 	-- An expiration of 0 never expires and is used for the redirect token.
+	expiration = tonumber(expiration)
 	if expiration ~= 0 and expiration <= os.time() then
 		return nil
 	end
diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua
index 742eabe..48c2add 100644
--- a/tests/extensions/test-auth.lua
+++ b/tests/extensions/test-auth.lua
@@ -192,6 +192,7 @@ h.equals("a value signed for one field does not serve another", validate_value("
 h.equals("no cookie does not verify", validate_value("username", nil), nil)
 h.equals("a tiny cookie does not verify", validate_value("username", "ab"), nil)
 h.equals("a leading separator does not verify", validate_value("username", "|x|1|s|sig"), nil)
+h.equals("a trailing field does not verify", validate_value("username", cookie .. "|x"), nil)
 h.equals("an unsigned cookie does not verify",
 	validate_value("username", "username|alice|123|salt"), nil)
 h.equals("an exponent spelling of the expiry does not verify",
-- 
cgit