Skip to content

Fix IMPORTANT audit findings (fix-id-sleep-yes-audit) #15

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-id-sleep-yes-audit) #15

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
12 changes: 9 additions & 3 deletions src/common/time.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ pub fn parseTimeString(time_str: []const u8) !u64 {
return error.InvalidTimeFormat;
}

// GNU sleep accepts 'inf' and 'infinity' to mean sleep forever.
if (std.mem.eql(u8, time_str, "inf") or std.mem.eql(u8, time_str, "infinity")) {
return std.math.maxInt(u64);
}

// Find the unit suffix (if any)
var number_part = time_str;
var unit = TimeUnit.seconds; // default unit
Expand Down Expand Up @@ -182,12 +187,13 @@ test "parseTimeString - invalid formats" {
try testing.expectEqual(@as(u64, @intFromFloat(0.5 * std.time.ns_per_s)), try parseTimeString(".5"));
}

test "parseTimeString - reject NaN and Inf" {
test "parseTimeString - reject NaN and Inf (except GNU-compatible inf/infinity)" {
try testing.expectError(error.InvalidTimeFormat, parseTimeString("nan"));
try testing.expectError(error.InvalidTimeFormat, parseTimeString("NaN"));
try testing.expectError(error.InvalidTimeFormat, parseTimeString("inf"));
// GNU sleep accepts 'inf' and 'infinity' as meaning sleep forever
try testing.expectEqual(std.math.maxInt(u64), try parseTimeString("inf"));
try testing.expectError(error.InvalidTimeFormat, parseTimeString("Inf"));
try testing.expectError(error.InvalidTimeFormat, parseTimeString("infinity"));
try testing.expectEqual(std.math.maxInt(u64), try parseTimeString("infinity"));
try testing.expectError(error.InvalidTimeFormat, parseTimeString("nans"));
try testing.expectError(error.InvalidTimeFormat, parseTimeString("infm"));
}
Expand Down
43 changes: 25 additions & 18 deletions src/id.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ pub fn runId(allocator: Allocator, args: []const []const u8, stdout_writer: anyt
return @intFromEnum(common.ExitCode.misuse);
}

// -a is a compatibility alias for -G (show all groups)
const show_groups = parsed.groups or parsed.all;
// GNU id: -a is a no-op (ignored), not an alias for -G
const show_groups = parsed.groups;

// -n and -r only make sense with -u, -g, or -G
if (parsed.name and !parsed.user and !parsed.group and !show_groups) {
Expand All @@ -147,7 +147,13 @@ pub fn runId(allocator: Allocator, args: []const []const u8, stdout_writer: anyt
return @intFromEnum(common.ExitCode.misuse);
}

// Mutually exclusive: -u, -g, -G/-a
// GNU id: -z requires a format flag (-u, -g, or -G)
if (parsed.zero and !parsed.user and !parsed.group and !show_groups) {
common.printErrorWithProgram(allocator, stderr_writer, "id", "option --zero not permitted in default format", .{});
return @intFromEnum(common.ExitCode.misuse);
}

// Mutually exclusive: -u, -g, -G
const mode_count: u8 = @as(u8, @intFromBool(parsed.user)) +
@as(u8, @intFromBool(parsed.group)) +
@as(u8, @intFromBool(show_groups));
Expand Down Expand Up @@ -303,7 +309,7 @@ pub fn runId(allocator: Allocator, args: []const []const u8, stdout_writer: anyt
return @intFromEnum(common.ExitCode.success);
}

// Handle -G (all groups) or -a (compatibility alias)
// Handle -G (all groups)
if (show_groups) {
const group_separator: u8 = if (parsed.zero) 0 else ' ';

Expand Down Expand Up @@ -1050,32 +1056,33 @@ test "id -p does not contain uid= format" {
try testing.expect(std.mem.indexOf(u8, stdout_buffer.items, "uid=") == null);
}

test "id -a shows all groups (same as -G)" {
test "id -a is a no-op (GNU behavior), produces default format" {
var stdout_a = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
defer stdout_a.deinit(testing.allocator);
var stdout_g = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
defer stdout_g.deinit(testing.allocator);
var stdout_default = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
defer stdout_default.deinit(testing.allocator);

const args_a = [_][]const u8{"-a"};
const result_a = try runId(testing.allocator, &args_a, stdout_a.writer(testing.allocator), common.null_writer);
try testing.expectEqual(@as(u8, 0), result_a);

const args_g = [_][]const u8{"-G"};
const result_g = try runId(testing.allocator, &args_g, stdout_g.writer(testing.allocator), common.null_writer);
try testing.expectEqual(@as(u8, 0), result_g);
const args_default = [_][]const u8{};
const result_default = try runId(testing.allocator, &args_default, stdout_default.writer(testing.allocator), common.null_writer);
try testing.expectEqual(@as(u8, 0), result_default);

// -a and -G should produce identical output
try testing.expectEqualStrings(stdout_g.items, stdout_a.items);
// GNU: -a is a no-op, so output should match plain id
try testing.expectEqualStrings(stdout_default.items, stdout_a.items);
}

test "id -a with -n shows group names" {
var stdout_buffer = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
defer stdout_buffer.deinit(testing.allocator);
test "id -a with -n is rejected (GNU: -a is no-op, -n alone is invalid)" {
var stderr_buffer = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
defer stderr_buffer.deinit(testing.allocator);

const args = [_][]const u8{ "-a", "-n" };
const result = try runId(testing.allocator, &args, stdout_buffer.writer(testing.allocator), common.null_writer);
try testing.expectEqual(@as(u8, 0), result);
try testing.expect(stdout_buffer.items.len > 1);
const result = try runId(testing.allocator, &args, common.null_writer, stderr_buffer.writer(testing.allocator));
// -a is no-op, -n without -u/-g/-G is an error
try testing.expectEqual(@as(u8, 2), result);
try testing.expect(stderr_buffer.items.len > 0);
}

test "id -A prints audit stub and exits 1" {
Expand Down
33 changes: 27 additions & 6 deletions src/sleep.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ fn parseTotalTime(args: []const []const u8) !u64 {
return total_nanos;
}

/// Find the first token that fails to parse as a valid time string.
/// Used to include the offending token in error messages (GNU behavior).
fn findBadToken(args: []const []const u8) ?[]const u8 {
for (args) |arg| {
_ = time.parseTimeString(arg) catch return arg;
}
return null;
}

/// Print help message
fn printHelp(allocator: std.mem.Allocator, writer: anytype) !void {
try common.help.printColorized(allocator, writer,
Expand Down Expand Up @@ -99,7 +108,7 @@ pub fn runSleep(allocator: std.mem.Allocator, args: []const []const u8, stdout_w
return @intFromEnum(common.ExitCode.success);
}

// Parse time arguments
// Parse time arguments, tracking which token failed for error reporting
const total_nanos = parseTotalTime(parsed_args.positionals) catch |err| {
switch (err) {
error.MissingTimeArgument => {
Expand All @@ -108,11 +117,22 @@ pub fn runSleep(allocator: std.mem.Allocator, args: []const []const u8, stdout_w
return @intFromEnum(common.ExitCode.misuse);
},
error.InvalidTimeFormat => {
common.printErrorWithProgram(allocator, stderr_writer, "sleep", "invalid time interval", .{});
// Find the offending token for the error message (GNU includes it)
const bad_token = findBadToken(parsed_args.positionals);
if (bad_token) |token| {
common.printErrorWithProgram(allocator, stderr_writer, "sleep", "invalid time interval '{s}'", .{token});
} else {
common.printErrorWithProgram(allocator, stderr_writer, "sleep", "invalid time interval", .{});
}
return @intFromEnum(common.ExitCode.misuse);
},
error.NegativeTime => {
common.printErrorWithProgram(allocator, stderr_writer, "sleep", "invalid time interval", .{});
const bad_token = findBadToken(parsed_args.positionals);
if (bad_token) |token| {
common.printErrorWithProgram(allocator, stderr_writer, "sleep", "invalid time interval '{s}'", .{token});
} else {
common.printErrorWithProgram(allocator, stderr_writer, "sleep", "invalid time interval", .{});
}
return @intFromEnum(common.ExitCode.misuse);
},
error.TimeOverflow => {
Expand Down Expand Up @@ -213,12 +233,13 @@ test "parseTimeString - invalid formats" {
try testing.expectEqual(@as(u64, @intFromFloat(0.5 * std.time.ns_per_s)), try time.parseTimeString(".5"));
}

test "parseTimeString - reject NaN and Inf" {
test "parseTimeString - reject NaN and Inf (except GNU-compatible inf/infinity)" {
try testing.expectError(error.InvalidTimeFormat, time.parseTimeString("nan"));
try testing.expectError(error.InvalidTimeFormat, time.parseTimeString("NaN"));
try testing.expectError(error.InvalidTimeFormat, time.parseTimeString("inf"));
// GNU sleep accepts 'inf' and 'infinity' as meaning sleep forever
try testing.expectEqual(std.math.maxInt(u64), try time.parseTimeString("inf"));
try testing.expectError(error.InvalidTimeFormat, time.parseTimeString("Inf"));
try testing.expectError(error.InvalidTimeFormat, time.parseTimeString("infinity"));
try testing.expectEqual(std.math.maxInt(u64), try time.parseTimeString("infinity"));
try testing.expectError(error.InvalidTimeFormat, time.parseTimeString("nans"));
try testing.expectError(error.InvalidTimeFormat, time.parseTimeString("infm"));
}
Expand Down
43 changes: 41 additions & 2 deletions src/yes.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@ pub fn runYes(
const parsed_args = common.argparse.ArgParser.parse(YesArgs, allocator, args) catch |err| {
switch (err) {
error.UnknownFlag => {
common.printErrorWithProgram(allocator, stderr_writer, "yes", "unrecognized option", .{});
return @intFromEnum(common.ExitCode.misuse);
// GNU yes exits 1 for unrecognized options and includes the flag name
const bad_flag = findUnknownFlag(args);
if (bad_flag) |flag| {
common.printErrorWithProgram(allocator, stderr_writer, "yes", "unrecognized option '{s}'", .{flag});
} else {
common.printErrorWithProgram(allocator, stderr_writer, "yes", "unrecognized option", .{});
}
return @intFromEnum(common.ExitCode.general_error);
},
error.MissingValue => {
common.printErrorWithProgram(allocator, stderr_writer, "yes", "option requires an argument", .{});
Expand Down Expand Up @@ -102,6 +108,39 @@ pub fn runYes(
}
}

/// Find the first unrecognized flag in args for error reporting.
/// Returns the full flag string (e.g., "--bad-flag") or null.
fn findUnknownFlag(args: []const []const u8) ?[]const u8 {
for (args) |arg| {
if (arg.len > 1 and arg[0] == '-') {
if (std.mem.eql(u8, arg, "--")) break;
// Check if it's a known flag
if (arg.len > 2 and arg[1] == '-') {
// Long flag
const flag_content = arg[2..];
const flag_name = if (std.mem.indexOfScalar(u8, flag_content, '=')) |eq_pos|
flag_content[0..eq_pos]
else
flag_content;
if (std.mem.eql(u8, flag_name, "help") or
std.mem.eql(u8, flag_name, "version"))
{
continue;
}
return arg;
} else {
// Short flag - check each character
for (arg[1..]) |c| {
if (c != 'h' and c != 'V') {
return arg;
}
}
}
}
}
return null;
}

/// Prints help text for the yes utility.
fn printHelp(allocator: std.mem.Allocator, writer: anytype) !void {
try common.help.printColorized(allocator, writer,
Expand Down
Loading