From e1996ad1e5c7c39a0ce5cfd0d3495a9cb58a00b2 Mon Sep 17 00:00:00 2001
From: Jonathan Marler <johnnymarler@gmail.com>
Date: Sat, 10 Feb 2024 21:09:05 -0700
Subject: [PATCH] os: remove UB, tmpDir is returning stack memory on Windows

On Windows, the tmpDir function is currently using a buffer on the stack
to convert the WTF16-encoded environment variable value "TMP" to utf8
and then returns it as a slice...but that stack buffer is no longer valid
when the function returns.  This was causing the "image load...temporary
file" test to fail on Windows.

I've updated the function to take an allocator but it only uses
the allocator on Windows.  No allocation is needed on other platforms
because they return environment variables that are already utf8 (ascii)
encoded, and the OS pre-allocates all environment variables in the process.
To keep the conditional that determines when allocation is required, I
added the `freeTmpDir` function.
---
 src/os/TempDir.zig                    |  3 ++-
 src/os/file.zig                       | 20 ++++++++++++--------
 src/terminal/kitty/graphics_image.zig |  3 ++-
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/os/TempDir.zig b/src/os/TempDir.zig
index 7c1354bae..7d3a34c84 100644
--- a/src/os/TempDir.zig
+++ b/src/os/TempDir.zig
@@ -31,7 +31,8 @@ pub fn init() !TempDir {
 
     const dir = dir: {
         const cwd = std.fs.cwd();
-        const tmp_dir = internal_os.tmpDir() orelse break :dir cwd;
+        const tmp_dir = internal_os.allocTmpDir(std.heap.page_allocator) orelse break :dir cwd;
+        defer internal_os.freeTmpDir(std.heap.page_allocator, tmp_dir);
         break :dir try cwd.openDir(tmp_dir, .{});
     };
 
diff --git a/src/os/file.zig b/src/os/file.zig
index ee27addb0..e48524cdd 100644
--- a/src/os/file.zig
+++ b/src/os/file.zig
@@ -44,22 +44,26 @@ pub fn fixMaxFiles() void {
 }
 
 /// Return the recommended path for temporary files.
-pub fn tmpDir() ?[]const u8 {
+/// This may not actually allocate memory, use freeTmpDir to properly
+/// free the memory when applicable.
+pub fn allocTmpDir(allocator: std.mem.Allocator) ?[]const u8 {
     if (builtin.os.tag == .windows) {
         // TODO: what is a good fallback path on windows?
         const v = std.os.getenvW(std.unicode.utf8ToUtf16LeStringLiteral("TMP")) orelse return null;
-        // MAX_PATH is very likely sufficient, but it's theoretically possible for someone to
-        // configure their os to allow paths as big as std.os.windows.PATH_MAX_WIDE, which is MUCH
-        // larger. Even if they did that, though, it's very unlikey that their Temp dir will use
-        // such a long path. We can switch if we see any issues, though it seems fairly unlikely.
-        var buf = [_]u8{0} ** std.os.windows.MAX_PATH;
-        const len = std.unicode.utf16leToUtf8(buf[0..], v[0..v.len]) catch |e| {
+        return std.unicode.utf16leToUtf8Alloc(allocator, v) catch |e| {
             log.warn("failed to convert temp dir path from windows string: {}", .{e});
             return null;
         };
-        return buf[0..len];
     }
     if (std.os.getenv("TMPDIR")) |v| return v;
     if (std.os.getenv("TMP")) |v| return v;
     return "/tmp";
 }
+
+/// Free a path returned by tmpDir if it allocated memory.
+/// This is a "no-op" for all platforms except windows.
+pub fn freeTmpDir(allocator: std.mem.Allocator, dir: []const u8) void {
+    if (builtin.os.tag == .windows) {
+        allocator.free(dir);
+    }
+}
diff --git a/src/terminal/kitty/graphics_image.zig b/src/terminal/kitty/graphics_image.zig
index 34dd2011f..fa20eaa10 100644
--- a/src/terminal/kitty/graphics_image.zig
+++ b/src/terminal/kitty/graphics_image.zig
@@ -192,7 +192,8 @@ pub const LoadingImage = struct {
     fn isPathInTempDir(path: []const u8) bool {
         if (std.mem.startsWith(u8, path, "/tmp")) return true;
         if (std.mem.startsWith(u8, path, "/dev/shm")) return true;
-        if (internal_os.tmpDir()) |dir| {
+        if (internal_os.allocTmpDir(std.heap.page_allocator)) |dir| {
+            defer internal_os.freeTmpDir(std.heap.page_allocator, dir);
             if (std.mem.startsWith(u8, path, dir)) return true;
 
             // The temporary dir is sometimes a symlink. On macOS for