feat: add path safety and sanitization methods to ZipEntry

* Added `ZipEntry:getSafePath` which returns the path of the entry if it
  was safe, or nil
* Added `ZipEntry:sanitizePath` which converts a potentially unsafe path
  to a safe one, although it can change the meaning of the paths
* Updated path utility with functions `isSafe` and `sanitize`
* Path utility now always uses `/` as a path separator, converting `\\`
  to `/` when needed
* Included tests for path utility

lib/init.luau

@@ -46,30 +46,28 @@ local EMPTY_PROPERTIES: ZipEntryProperties = table.freeze({
 })
 
 local MADE_BY_OS_LOOKUP: { [number]: MadeByOS } = {
-    [0x0] = "FAT",
-    [0x1] = "AMIGA", 
-    [0x2] = "VMS",
-    [0x3] = "UNIX",
-    [0x4] = "VM/CMS",
-    [0x5] = "Atari ST",
-    [0x6] = "OS/2",
-    [0x7] = "MAC",
-    [0x8] = "Z-System",
-    [0x9] = "CP/M",
-    [0xa] = "NTFS",
-    [0xb] = "MVS",
-    [0xc] = "VSE",
-    [0xd] = "Acorn RISCOS",
-    [0xe] = "VFAT",
-    [0xf] = "Alternate MVS",
-    [0x10] = "BeOS",
-    [0x11] = "TANDEM",
-    [0x12] = "OS/400",
-    [0x13] = "OS/X",
+	[0x0] = "FAT",
+	[0x1] = "AMIGA",
+	[0x2] = "VMS",
+	[0x3] = "UNIX",
+	[0x4] = "VM/CMS",
+	[0x5] = "Atari ST",
+	[0x6] = "OS/2",
+	[0x7] = "MAC",
+	[0x8] = "Z-System",
+	[0x9] = "CP/M",
+	[0xa] = "NTFS",
+	[0xb] = "MVS",
+	[0xc] = "VSE",
+	[0xd] = "Acorn RISCOS",
+	[0xe] = "VFAT",
+	[0xf] = "Alternate MVS",
+	[0x10] = "BeOS",
+	[0x11] = "TANDEM",
+	[0x12] = "OS/400",
+	[0x13] = "OS/X",
 }
 
--- TODO: ERROR HANDLING!
-
 local ZipEntry = {}
 export type ZipEntry = typeof(setmetatable({} :: ZipEntryInner, { __index = ZipEntry }))
 -- stylua: ignore
@@ -129,7 +127,7 @@ export type ZipEntryProperties = {
 function ZipEntry.new(offset: number, name: string, properties: ZipEntryProperties): ZipEntry
 	local versionMadeByOS = bit32.rshift(properties.versionMadeBy, 8)
 	local versionMadeByVersion = bit32.band(properties.versionMadeBy, 0x00ff)
-	
+
 	return setmetatable(
 		{
 			name = name,
@@ -166,6 +164,21 @@ function ZipEntry.getPath(self: ZipEntry): string
 	return path
 end
 
+function ZipEntry.getSafePath(self: ZipEntry): string?
+	local pathStr = self:getPath()
+
+	if path.isSafe(pathStr) then
+		return pathStr
+	end
+
+	return nil
+end
+
+function ZipEntry.sanitizePath(self: ZipEntry): string
+	local pathStr = self:getPath()
+	return path.sanitize(pathStr)
+end
+
 -- TODO: More methods for `ZipEntry`, handle octals and unix perms
 
 local ZipReader = {}

lib/utils/path.luau

@@ -1,8 +1,7 @@
 --- Canonicalize a path by removing redundant components
 local function canonicalize(path: string): string
-	-- NOTE: It is fine for us to use `/` here because ZIP file names
-	-- always use `/` as the path separator
-	local components = string.split(path, "/")
+	-- We convert any `\\` separators to `/` separators to ensure consistency
+	local components = string.split(path:gsub("\\", "/"), "/")
 	local result = {}
 	for _, component in components do
 		if component == "." then
@@ -25,16 +24,101 @@ end
 
 --- Check if a path is absolute
 local function isAbsolute(path: string): boolean
-	return (string.match(path, "^/") or string.match(path, "^[a-zA-Z]:[\\/]") or string.match(path, "^//")) ~= nil
+	return (
+		string.match(path, "^/")
+		or string.match(path, "^[a-zA-Z]:[\\/]")
+		or string.match(path, "^//")
+		or string.match(path, "^\\\\")
+	) ~= nil
 end
 
 --- Check if a path is relative
 local function isRelative(path: string): boolean
 	return not isAbsolute(path)
 end
 
+local function replaceBackslashes(input: string, replacement: "/"): string
+	-- Check if the string starts with double backslashes
+	if input:sub(1, 2) == "\\\\" then
+		-- Replace all single backslashes except the first two
+		return "\\\\" .. input:sub(3):gsub("\\", replacement)
+	else
+		-- Replace all single backslashes
+		return input:gsub("\\", replacement)
+	end
+end
+
+--- Check if a path is safe to use, i.e., it does not:
+--- - Contain null bytes
+--- - Resolve to a directory outside of the current directory
+--- - Have absolute components
+local function isSafe(path: string): boolean
+	if string.find(path, "\0") or isAbsolute(path) then
+		-- Null bytes or absolute path, path is unsafe
+		return false
+	end
+
+	local components = string.split(replaceBackslashes(path, "/"), "/")
+	local depth = 0
+	for _, component in components do
+		if string.match(component, "^[a-zA-Z]:$") or string.find(component, "^\\\\") or component == "" then
+			-- Was a prefix component, or the root directory, path is unsafe
+			return false
+		end
+
+		if component == ".." then
+			-- Traverse one upwards
+			depth -= 1
+			if depth < 0 then
+				-- Traversed too far, path is unsafe
+				return false
+			end
+
+			continue
+		end
+
+		if component == "." then
+			-- Skip current directory
+			continue
+		end
+
+		-- Otherwise, increment depth
+		depth += 1
+	end
+
+	return depth >= 0
+end
+
+--- Sanitize a path by ignoring special components
+--- - Absolute paths become relative
+--- - Special components (like upwards traversing) are removed
+--- - Truncates path to the first null byte
+local function sanitize(path: string): string
+	local truncatedPath = if string.find(path, "\0") then string.split(path, "\0")[1] else path
+	local components = string.split(replaceBackslashes(truncatedPath, "/"), "/")
+	local result = {}
+	for _, component in components do
+		if
+			not (
+				component == ".."
+				or component == "."
+				or component == string.match(component, "^[a-zA-Z]:$")
+				or string.find(component, "^\\\\")
+				or component == ""
+			)
+		then
+			-- If the path is not a special component, add it to the result
+			table.insert(result, component)
+		end
+	end
+
+	return table.concat(result, "/")
+end
+
 return {
 	canonicalize = canonicalize,
 	isAbsolute = isAbsolute,
 	isRelative = isRelative,
+	isSafe = isSafe,
+	sanitize = sanitize,
 }

tests/path.luau

@@ -0,0 +1,83 @@
+local frktest = require("../lune_packages/frktest")
+local check = frktest.assert.check
+
+local path = require("../lib/utils/path")
+
+return function(test: typeof(frktest.test))
+	test.suite("Path utilities function as expected", function()
+		test.case("Canonicalize should handle basic paths", function()
+			check.equal(path.canonicalize("foo/bar"), "foo/bar")
+			check.equal(path.canonicalize("foo\\bar"), "foo/bar")
+		end)
+
+		test.case("Canonicalize should remove current directory markers", function()
+			check.equal(path.canonicalize("foo/./bar"), "foo/bar")
+			check.equal(path.canonicalize("./foo/bar"), "foo/bar")
+		end)
+
+		test.case("Canonicalize should handle parent directory traversal", function()
+			check.equal(path.canonicalize("foo/bar/../baz"), "foo/baz")
+			check.equal(path.canonicalize("foo/bar/../../baz"), "baz")
+		end)
+
+		test.case("isAbsolute should identify Unix-style absolute paths", function()
+			check.is_true(path.isAbsolute("/foo/bar"))
+			check.is_true(path.isAbsolute("//foo/bar"))
+		end)
+
+		test.case("isAbsolute should identify Windows-style absolute paths", function()
+			check.is_true(path.isAbsolute("C:\\foo\\bar"))
+			check.is_true(path.isAbsolute("\\\\server\\share"))
+		end)
+
+		test.case("isAbsolute should identify relative paths", function()
+			check.is_false(path.isAbsolute("foo/bar"))
+			check.is_false(path.isAbsolute("./foo/bar"))
+		end)
+
+		test.case("isRelative should be inverse of isAbsolute", function()
+			check.is_false(path.isRelative("/foo/bar"))
+			check.is_false(path.isRelative("C:\\foo\\bar"))
+			check.is_true(path.isRelative("foo/bar"))
+			check.is_true(path.isRelative("./foo/bar"))
+		end)
+
+		test.case("isSafe should reject paths with null bytes", function()
+			check.is_false(path.isSafe("foo\0bar"))
+		end)
+
+		test.case("isSafe should reject absolute paths", function()
+			check.is_false(path.isSafe("/foo/bar"))
+			check.is_false(path.isSafe("C:\\foo\\bar"))
+		end)
+
+		test.case("isSafe should reject paths that escape current directory", function()
+			check.is_false(path.isSafe("../foo"))
+			check.is_false(path.isSafe("foo/../../bar"))
+		end)
+
+		test.case("isSafe should accept safe relative paths", function()
+			check.is_true(path.isSafe("foo/bar"))
+			check.is_true(path.isSafe("foo/bar/baz"))
+		end)
+
+		test.case("Sanitize should remove special components", function()
+			check.equal(path.sanitize("../foo/bar"), "foo/bar")
+			check.equal(path.sanitize("./foo/bar"), "foo/bar")
+			check.equal(path.sanitize("C:\\foo\\bar"), "foo/bar")
+			check.equal(path.sanitize("\\\\server\\share\\foo"), "share/foo")
+		end)
+
+		test.case("Sanitize should truncate at null bytes", function()
+			check.equal(path.sanitize("foo\0bar/baz"), "foo")
+		end)
+
+		test.case("Sanitize should convert backslashes to forward slashes", function()
+			check.equal(path.sanitize("foo\\bar\\baz"), "foo/bar/baz")
+		end)
+
+		test.case("Sanitize should handle empty components", function()
+			check.equal(path.sanitize("/foo//bar"), "foo/bar")
+		end)
+	end)
+end