Skip to content

Commit a4bd4e7

Browse files
committed
fix: 4 more issues from deep audit (argon2id bounds, arithmetic overflow, NaN bypass, unreachable FFI field)
Continuing the audit sweep into cap:ffi, the remaining capability modules, copy-paste-divergence patterns, and numeric edge cases. - crypto.zig argon2id: t/m/p validated only lower bounds (unlike bcryptHash's cost, bounded both ways) before @intCast-ing them into argon2.Params' u32/u32/u24 fields -- an out-of-range value panicked that cast immediately, a crash trivially reachable from Gengo source. - vm.zig .mod opcode: missing the minInt(i64)/-1 overflow guard its siblings .int_div/.rem already have (a copy-paste divergence found by audit) -- @mod traps/UB's on the same 2^63 overflow. Also fixed .int_div's OTHER path (named/decimal-typed operands, reached via numericBinaryOp) which silently returned minInt(i64) instead of erroring like its plain-int sibling already does. - cap_net.zig extractHandle/extractUsize/extractI64: NaN compares false against every ordinary comparison, so `n < 0 or n > MAX` guards silently passed NaN through to @intFromFloat -- the same hazard floatToIntSafe (vm.zig) exists to close, reimplemented here without that check. Added isFinite guards to all three. - module_compile.zig: ffi.buf_from_ptr was fully implemented (cap_ffi.zig) but permanently unreachable -- missing from the compiler's cap:ffi field allowlist, so any script referencing it failed to compile at all. Verified fixed via the CLI. Verified clean under standard, -Dpreset=stress, and -Dgc_stress=true.
1 parent c92071c commit a4bd4e7

4 files changed

Lines changed: 44 additions & 10 deletions

File tree

src/lang/module_compile.zig

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -962,12 +962,17 @@ pub fn hasModuleExport(ctx: *anyopaque, path: []const u8, field: []const u8) boo
962962
const idx = self.findModule(path) orelse {
963963
// Check capability modules
964964
const cap_key = if (std.mem.startsWith(u8, path, "cap:")) path[4..] else path;
965-
// cap:ffi additionally exports "types" (ffi.types.i32 etc.) and "buf"
966-
// (ffi.buf(n)) that are not functions in cm.functions. They are
967-
// installed at runtime by installFfiModule in native/main.zig.
965+
// cap:ffi additionally exports "types" (ffi.types.i32 etc.), "buf"
966+
// (ffi.buf(n)), and "buf_from_ptr" (ffi.buf_from_ptr(ptr, len)) that
967+
// are not functions in cm.functions. They are installed at runtime
968+
// by installFfiModule in native/main.zig. buf_from_ptr was missing
969+
// from this allowlist -- its runtime dispatch (cap_ffi.zig) was
970+
// fully implemented but permanently unreachable, since any script
971+
// referencing ffi.buf_from_ptr failed to compile at all
972+
// (UnknownField) before ever reaching it.
968973
if (comptime build_options.cap_ffi) {
969974
if (common.streq(cap_key, "ffi") and
970-
(common.streq(field, "types") or common.streq(field, "buf"))) return true;
975+
(common.streq(field, "types") or common.streq(field, "buf") or common.streq(field, "buf_from_ptr"))) return true;
971976
}
972977
for (self.capability_modules) |cm| {
973978
if (common.streq(cm.name, cap_key)) {

src/lang/native/cap_net.zig

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,14 @@ fn extractHandle(arg: Value) !u32 {
2727
break :blk @intCast(n);
2828
},
2929
.float => |n| blk: {
30-
if (n < 0 or n > @as(f64, @floatFromInt(std.math.maxInt(u32)))) return error.TypeError;
30+
// !isFinite catches NaN too, not just checking bounds: NaN
31+
// compares false against every ordinary comparison (IEEE 754),
32+
// so a bare `n < 0 or n > MAX` guard silently passes NaN
33+
// through to @intFromFloat below, which is safety-checked UB
34+
// for a non-finite input — the same hazard floatToIntSafe
35+
// (vm.zig) exists specifically to close, reimplemented here
36+
// without that check.
37+
if (!std.math.isFinite(n) or n < 0 or n > @as(f64, @floatFromInt(std.math.maxInt(u32)))) return error.TypeError;
3138
break :blk @as(u32, @intFromFloat(n));
3239
},
3340
else => return error.TypeError,
@@ -41,7 +48,7 @@ fn extractUsize(arg: Value) !usize {
4148
break :blk @as(usize, @intCast(n));
4249
},
4350
.float => |n| blk: {
44-
if (n < 0 or n > @as(f64, @floatFromInt(std.math.maxInt(usize)))) return error.TypeError;
51+
if (!std.math.isFinite(n) or n < 0 or n > @as(f64, @floatFromInt(std.math.maxInt(usize)))) return error.TypeError;
4552
break :blk @as(usize, @intFromFloat(n));
4653
},
4754
else => return error.TypeError,
@@ -52,7 +59,7 @@ fn extractI64(arg: Value) !i64 {
5259
return switch (arg) {
5360
.int => |n| n,
5461
.float => |n| blk: {
55-
if (n < @as(f64, @floatFromInt(std.math.minInt(i64))) or n >= std.math.pow(f64, 2.0, 63.0)) return error.TypeError;
62+
if (!std.math.isFinite(n) or n < @as(f64, @floatFromInt(std.math.minInt(i64))) or n >= std.math.pow(f64, 2.0, 63.0)) return error.TypeError;
5663
break :blk @as(i64, @intFromFloat(n));
5764
},
5865
else => return error.TypeError,

src/lang/native/crypto.zig

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,16 @@ fn argon2id(ctx: VMContext, argc: u8) !Value {
453453
const p = try vms.valueAsInt(ctx.vs.stack[ctx.vs.stack_top - 2]);
454454
const key_len = try vms.valueAsInt(ctx.vs.stack[ctx.vs.stack_top - 1]);
455455

456-
if (t < 1 or m < 8 or p < 1 or key_len < 1 or key_len > 64) return error.ValueError;
456+
// Upper bounds (not just bcryptHash's lower-bound style) matter here:
457+
// t/m/p are cast into std.crypto.pwhash.argon2.Params' u32/u32/u24
458+
// fields below with @intCast, and an out-of-range i64 (e.g. m between
459+
// u32::max and i64::max) panics that cast immediately -- a crash
460+
// trivially reachable from Gengo source, no wire format or policy
461+
// misconfiguration needed. The bounds also cap real resource use: m is
462+
// in KiB, so 4 GiB and 100 iterations are already generous for any
463+
// legitimate password-hashing use, while still comfortably fitting
464+
// u32/u32/u24 (max ~4.29e9 / ~4.29e9 / ~1.68e7 respectively).
465+
if (t < 1 or t > 100 or m < 8 or m > 4 * 1024 * 1024 or p < 1 or p > 255 or key_len < 1 or key_len > 64) return error.ValueError;
457466
if (salt.len < 8) return error.ValueError;
458467

459468
// Copy inputs to stack buffers so GC allocation below is safe.

src/lang/vm.zig

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3298,7 +3298,16 @@ fn execOne(ctx: VMContext, comptime op: Op) anyerror!bool {
32983298
ctx.vs.setRuntimeErr("division by zero", .{});
32993299
return error.DivisionByZero;
33003300
}
3301-
const result: i64 = if (an_int == std.math.minInt(i64) and bn_int == -1) std.math.minInt(i64) else @divTrunc(an_int, bn_int);
3301+
// This carrier path (named/decimal-wrapped int operands) used
3302+
// to silently return minInt(i64) here instead of erroring —
3303+
// a mathematically wrong answer (the true result is 2^63, one
3304+
// past i64's range), reintroducing the exact bug the plain
3305+
// int/int path above was already fixed for.
3306+
if (an_int == std.math.minInt(i64) and bn_int == -1) {
3307+
ctx.vs.setRuntimeErr("integer overflow in division", .{});
3308+
return error.RangeError;
3309+
}
3310+
const result: i64 = @divTrunc(an_int, bn_int);
33023311
try pushNumericResultWithCarrier(ctx, a, b, @floatFromInt(result), nop.tag, "div");
33033312
},
33043313
.rem => {
@@ -3344,7 +3353,11 @@ fn execOne(ctx: VMContext, comptime op: Op) anyerror!bool {
33443353
ctx.vs.setRuntimeErr("division by zero", .{});
33453354
return error.DivisionByZero;
33463355
}
3347-
const result: i64 = @mod(a.int, b.int);
3356+
// Same minInt(i64)/-1 overflow as .int_div/.rem just above
3357+
// (2^63, one past maxInt) — @mod traps/UB's on it exactly
3358+
// like @divTrunc/@rem do, and x mod ±1 is always 0
3359+
// mathematically, matching .rem's guard for this case.
3360+
const result: i64 = if (a.int == std.math.minInt(i64) and b.int == -1) 0 else @mod(a.int, b.int);
33483361
try ctx.vs.vmPush(.{ .int = result });
33493362
continue;
33503363
}

0 commit comments

Comments
 (0)