core: protect against crashes and hangs when themes are not files (#5632)

If a theme was not a file or a directory you could get a crash or a hang
(depending on platform) if the theme references a directory. This patch
also prevents attempts to load from other non-file sources.

Fixes: #5596
This commit is contained in:
Mitchell Hashimoto
2025-02-11 07:24:10 -08:00
committed by GitHub
2 changed files with 62 additions and 6 deletions

View File

@ -8,6 +8,8 @@ const internal_os = @import("../os/main.zig");
const Diagnostic = diags.Diagnostic;
const DiagnosticList = diags.DiagnosticList;
const log = std.log.scoped(.cli);
// TODO:
// - Only `--long=value` format is accepted. Do we want to allow
// `--long value`? Not currently allowed.
@ -1258,9 +1260,11 @@ pub fn LineIterator(comptime ReaderType: type) type {
const buf = buf: {
while (true) {
// Read the full line
var entry = self.r.readUntilDelimiterOrEof(self.entry[2..], '\n') catch {
// TODO: handle errors
unreachable;
var entry = self.r.readUntilDelimiterOrEof(self.entry[2..], '\n') catch |err| switch (err) {
inline else => |e| {
log.warn("cannot read from \"{s}\": {}", .{ self.filepath, e });
return null;
},
} orelse return null;
// Increment our line counter

View File

@ -104,6 +104,10 @@ pub const LocationIterator = struct {
/// Due to the way allocations are handled, an Arena allocator (or another
/// similar allocator implementation) should be used. It may not be safe to
/// free the returned allocations.
///
/// This will never return anything other than a handle to a regular file. If
/// the theme resolves to something other than a regular file a diagnostic entry
/// will be added to the list and null will be returned.
pub fn open(
arena_alloc: Allocator,
theme: []const u8,
@ -119,6 +123,29 @@ pub fn open(
theme,
diags,
) orelse return null;
const stat = file.stat() catch |err| {
try diags.append(arena_alloc, .{
.message = try std.fmt.allocPrintZ(
arena_alloc,
"not reading theme from \"{s}\": {}",
.{ theme, err },
),
});
return null;
};
switch (stat.kind) {
.file => {},
else => {
try diags.append(arena_alloc, .{
.message = try std.fmt.allocPrintZ(
arena_alloc,
"not reading theme from \"{s}\": it is a {s}",
.{ theme, @tagName(stat.kind) },
),
});
return null;
},
}
return .{ .path = theme, .file = file };
}
@ -140,9 +167,34 @@ pub fn open(
const cwd = std.fs.cwd();
while (try it.next()) |loc| {
const path = try std.fs.path.join(arena_alloc, &.{ loc.dir, theme });
if (cwd.openFile(path, .{})) |file| return .{
.path = path,
.file = file,
if (cwd.openFile(path, .{})) |file| {
const stat = file.stat() catch |err| {
try diags.append(arena_alloc, .{
.message = try std.fmt.allocPrintZ(
arena_alloc,
"not reading theme from \"{s}\": {}",
.{ theme, err },
),
});
return null;
};
switch (stat.kind) {
.file => {},
else => {
try diags.append(arena_alloc, .{
.message = try std.fmt.allocPrintZ(
arena_alloc,
"not reading theme from \"{s}\": it is a {s}",
.{ theme, @tagName(stat.kind) },
),
});
return null;
},
}
return .{
.path = path,
.file = file,
};
} else |err| switch (err) {
// Not an error, just continue to the next location.
error.FileNotFound => {},