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.

2 files changed, 10 insertions, 82 deletions

diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.luaindex 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-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.luaindex 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/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