Skip to content

Fix IMPORTANT audit findings (fix-env-free-audit-tests) #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kelp wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix IMPORTANT audit findings (fix-env-free-audit-tests) #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 34 additions & 16 deletions src/env.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ pub fn runEnv(allocator: Allocator, args: []const []const u8, stdout_writer: any
return 0;
}

// Per macOS spec: -0 and utility may not be specified together
if (options.null_delimiter and options.command.len > 0) {
common.printErrorWithProgram(allocator, stderr_writer, "env", "cannot specify -0 with a utility", .{});
return 125;
}

// Handle -S: split string into tokens, process as assignments/command
if (options.split_string) |split_str| {
var split_assignments = std.ArrayListUnmanaged(Assignment){};
Expand Down Expand Up @@ -198,17 +204,6 @@ pub fn runEnv(allocator: Allocator, args: []const []const u8, stdout_writer: any
}
}

// Handle -P: set alternate PATH for command search
if (options.alt_path) |alt| {
if (options.verbose) {
stderr_writer.print("env: using alternate PATH: {s}\n", .{alt}) catch {};
}
env_map.put("PATH", alt) catch {
common.printErrorWithProgram(allocator, stderr_writer, "env", "failed to set alternate PATH", .{});
return @intFromEnum(common.ExitCode.general_error);
};
}

// Handle -C/--chdir
if (options.chdir) |dir| {
std.posix.chdir(dir) catch |err| {
Expand All @@ -225,6 +220,20 @@ pub fn runEnv(allocator: Allocator, args: []const []const u8, stdout_writer: any
return 0;
}

// Handle -P: set alternate PATH for command search.
// Per macOS spec, -P only affects where the utility is searched,
// not the child's environment. We set PATH in env_map here only
// for process.Child's search; it does not apply when printing.
if (options.alt_path) |alt| {
if (options.verbose) {
stderr_writer.print("env: using alternate PATH: {s}\n", .{alt}) catch {};
}
env_map.put("PATH", alt) catch {
common.printErrorWithProgram(allocator, stderr_writer, "env", "failed to set alternate PATH", .{});
return @intFromEnum(common.ExitCode.general_error);
};
}

// Execute the command
return execCommand(allocator, options.command, &env_map, stderr_writer);
}
Expand All @@ -238,6 +247,7 @@ fn parseArgs(allocator: Allocator, args: []const []const u8) error{ UnknownFlag,
errdefer assignments.deinit(allocator);
var i: usize = 0;
var past_options = false;
var seen_assignment = false;

while (i < args.len) : (i += 1) {
const arg = args[i];
Expand All @@ -260,20 +270,28 @@ fn parseArgs(allocator: Allocator, args: []const []const u8) error{ UnknownFlag,
continue;
}

// Check for NAME=VALUE assignment
// Check for NAME=VALUE assignment or command start
if (!past_options and !std.mem.startsWith(u8, arg, "-")) {
if (std.mem.indexOfScalar(u8, arg, '=')) |eq_pos| {
assignments.append(allocator, .{
.name = arg[0..eq_pos],
.value = arg[eq_pos + 1 ..],
}) catch return error.OutOfMemory;
seen_assignment = true;
continue;
}
// Not an assignment and not a flag -- it's the command
options.command = args[i..];
break;
}

// Per POSIX/macOS spec: once a NAME=VALUE has been seen,
// subsequent flag-like tokens start the command.
if (seen_assignment) {
options.command = args[i..];
break;
}

// Long flags
if (std.mem.startsWith(u8, arg, "--")) {
if (std.mem.eql(u8, arg, "--help")) {
Expand Down Expand Up @@ -927,17 +945,17 @@ test "env parseArgs: -v flag" {
try testing.expect(options.verbose);
}

test "env runEnv: -P sets alternate PATH in environment" {
test "env runEnv: -P does not set PATH when printing environment" {
var stdout_buffer = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
defer stdout_buffer.deinit(testing.allocator);
var stderr_buffer = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
defer stderr_buffer.deinit(testing.allocator);

const exit_code = runEnv(testing.allocator, &.{ "-i", "-P", "/custom/path", "FOO=bar" }, stdout_buffer.writer(testing.allocator), stderr_buffer.writer(testing.allocator));
try testing.expectEqual(@as(u8, 0), exit_code);
// Environment should contain the alternate PATH
try testing.expect(std.mem.indexOf(u8, stdout_buffer.items, "PATH=/custom/path\n") != null);
// And the assignment
// Per spec, -P only affects utility search path, not the environment
try testing.expect(std.mem.indexOf(u8, stdout_buffer.items, "PATH=") == null);
// Assignment should still appear
try testing.expect(std.mem.indexOf(u8, stdout_buffer.items, "FOO=bar\n") != null);
}

Expand Down
18 changes: 12 additions & 6 deletions src/free.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ const FreeArgs = struct {
.mebi = .{ .short = 'm', .desc = "Display output in mebibytes" },
.gibi = .{ .short = 'g', .desc = "Display output in gibibytes" },
.human = .{ .short = 'h', .desc = "Show human-readable output" },
.si = .{ .desc = "Use powers of 1000 instead of 1024" },
.si = .{ .short = 0, .desc = "Use powers of 1000 instead of 1024" },
.total = .{ .short = 't', .desc = "Display a line showing column totals" },
.wide = .{ .short = 'w', .desc = "Wide output" },
.seconds = .{ .short = 's', .desc = "Continuous display every N seconds", .value_name = "N" },
Expand Down Expand Up @@ -309,10 +309,10 @@ fn printMemRow(writer: anytype, label: []const u8, info: MemInfo, unit: Unit, us
try printValue(writer, info.used, unit, use_si);
try printValue(writer, info.free, unit, use_si);
try printValue(writer, info.shared, unit, use_si);
// Split buff/cache into buffers and cache
// On macOS we don't have a clean split, so use 0 and buff_cache
try printValue(writer, 0, unit, use_si);
// Split buff/cache into buffers and cache.
// MemInfo merges these; show buff_cache as buffers, 0 as cache.
try printValue(writer, info.buff_cache, unit, use_si);
try printValue(writer, 0, unit, use_si);
try printValue(writer, info.available, unit, use_si);
try writer.writeAll("\n");
} else {
Expand All @@ -339,8 +339,8 @@ fn printTotalRow(writer: anytype, info: MemInfo, unit: Unit, use_si: bool, wide:
try printValue(writer, total_used, unit, use_si);
try printValue(writer, total_free, unit, use_si);
try printValue(writer, info.shared, unit, use_si);
try printValue(writer, 0, unit, use_si);
try printValue(writer, info.buff_cache, unit, use_si);
try printValue(writer, 0, unit, use_si);
try printValue(writer, info.available, unit, use_si);
try writer.writeAll("\n");
} else {
Expand Down Expand Up @@ -414,7 +414,13 @@ pub fn runFree(allocator: Allocator, args: []const []const u8, stdout_writer: an
const repeat_count = parsed.count orelse 0;
const interval = parsed.seconds orelse 0;

// If -c is given without -s, treat as a single display
// Per GNU free: -c requires -s
if (parsed.count != null and parsed.seconds == null) {
common.printErrorWithProgram(allocator, stderr_writer, prog_name, "-c requires -s option", .{});
return @intFromEnum(common.ExitCode.misuse);
}

// No continuous mode requested, display once
if (interval == 0) {
return displayOnce(stdout_writer, stderr_writer, allocator, unit, use_si, show_total, wide);
}
Expand Down
Loading