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.
This commit is contained in:
Jonathan Marler
2024-02-10 21:09:05 -07:00
parent 74c26d1ef0
commit e1996ad1e5
3 changed files with 16 additions and 10 deletions

View File

@ -31,7 +31,8 @@ pub fn init() !TempDir {
const dir = dir: { const dir = dir: {
const cwd = std.fs.cwd(); 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, .{}); break :dir try cwd.openDir(tmp_dir, .{});
}; };

View File

@ -44,22 +44,26 @@ pub fn fixMaxFiles() void {
} }
/// Return the recommended path for temporary files. /// 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) { if (builtin.os.tag == .windows) {
// TODO: what is a good fallback path on windows? // TODO: what is a good fallback path on windows?
const v = std.os.getenvW(std.unicode.utf8ToUtf16LeStringLiteral("TMP")) orelse return null; 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 return std.unicode.utf16leToUtf8Alloc(allocator, v) catch |e| {
// 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| {
log.warn("failed to convert temp dir path from windows string: {}", .{e}); log.warn("failed to convert temp dir path from windows string: {}", .{e});
return null; return null;
}; };
return buf[0..len];
} }
if (std.os.getenv("TMPDIR")) |v| return v; if (std.os.getenv("TMPDIR")) |v| return v;
if (std.os.getenv("TMP")) |v| return v; if (std.os.getenv("TMP")) |v| return v;
return "/tmp"; 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);
}
}

View File

@ -192,7 +192,8 @@ pub const LoadingImage = struct {
fn isPathInTempDir(path: []const u8) bool { fn isPathInTempDir(path: []const u8) bool {
if (std.mem.startsWith(u8, path, "/tmp")) return true; if (std.mem.startsWith(u8, path, "/tmp")) return true;
if (std.mem.startsWith(u8, path, "/dev/shm")) 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; if (std.mem.startsWith(u8, path, dir)) return true;
// The temporary dir is sometimes a symlink. On macOS for // The temporary dir is sometimes a symlink. On macOS for