From 053b5e3bddc086c43bc44e11a0106a2f15a0f3af Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 26 Mar 2026 20:02:33 -0700 Subject: [PATCH] Package.Manifest: add missing hash validation closes #31225 reverts e96d86064eb81977f254fa8f36481b7d150cb3b6 which was an inadequate attempt to address the same problem (lack of hash validation). --- src/Package.zig | 27 +++++++++++++++++++-------- src/Package/Fetch.zig | 2 +- src/Package/Manifest.zig | 12 +++++------- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/Package.zig b/src/Package.zig index 0dca095833..8fb9995bd8 100644 --- a/src/Package.zig +++ b/src/Package.zig @@ -44,6 +44,8 @@ pub const Fingerprint = packed struct(u64) { pub const Hash = struct { /// Maximum size of a package hash. Unused bytes at the end are /// filled with zeroes. + /// + /// Assumed to be already validated. bytes: [max_len]u8, pub const Algo = std.crypto.hash.sha2.Sha256; @@ -52,21 +54,35 @@ pub const Hash = struct { /// Example: "nnnn-vvvv-hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh" pub const max_len = 32 + 1 + 32 + 1 + (32 + 32 + 200) / 6; + /// Asserts `s` is valid. pub fn fromSlice(s: []const u8) Hash { - assert(s.len <= max_len); + assert(validate(s) == .ok); var result: Hash = undefined; @memcpy(result.bytes[0..s.len], s); @memset(result.bytes[s.len..], 0); return result; } + pub const Validation = enum { ok, short, long, incomplete }; + + pub fn validate(s: []const u8) Validation { + if (s.len > max_len) return .long; + if (s.len < 44) return .short; + const n_dashes = std.mem.countScalar(u8, s[0 .. s.len - 44], '-'); + if (n_dashes < 2) return .incomplete; + return .ok; + } + + test validate { + try std.testing.expectEqual(.short, validate("")); + } + pub fn toSlice(ph: *const Hash) []const u8 { var end: usize = ph.bytes.len; - while (end > 0) { + while (true) { end -= 1; if (ph.bytes[end] != 0) return ph.bytes[0 .. end + 1]; } - return ph.bytes[0..0]; } pub fn eql(a: *const Hash, b: *const Hash) bool { @@ -188,11 +204,6 @@ test Hash { try std.testing.expectEqualStrings("nasm-2.16.1-3-vrr-ygAAoADH9XG3tOdvPNuHen_d-XeHndOG-nNXmved", result.toSlice()); } -test "empty hash" { - const hash = Hash.fromSlice(""); - try std.testing.expectEqualStrings("", hash.toSlice()); -} - test { _ = Fetch; } diff --git a/src/Package/Fetch.zig b/src/Package/Fetch.zig index 0fc65670ca..139fba9d18 100644 --- a/src/Package/Fetch.zig +++ b/src/Package/Fetch.zig @@ -783,7 +783,7 @@ fn runResource( const hash_tok = f.hash_tok.unwrap().?; if (!computed_package_hash.eql(&declared_hash)) { return f.fail(hash_tok, try eb.printString( - "hash mismatch: manifest declares '{s}' but the fetched package has '{s}'", + "hash mismatch: manifest declares {s} but the fetched package has {s}", .{ declared_hash.toSlice(), computed_package_hash.toSlice() }, )); } diff --git a/src/Package/Manifest.zig b/src/Package/Manifest.zig index 90d61c963b..05ca2a31c2 100644 --- a/src/Package/Manifest.zig +++ b/src/Package/Manifest.zig @@ -420,12 +420,10 @@ const Parse = struct { const ast = p.ast; const tok = ast.nodeMainToken(node); const h = try parseString(p, node); - - if (h.len > Package.Hash.max_len) { - return fail(p, tok, "hash length exceeds maximum: {d}", .{h.len}); + switch (Package.Hash.validate(h)) { + .ok => return h, + else => |t| return fail(p, tok, "invalid hash: {t}", .{t}), } - - return h; } /// TODO: try to DRY this with AstGen.identifierTokenString @@ -632,7 +630,7 @@ test "basic" { \\ .dependencies = .{ \\ .bar = .{ \\ .url = "https://example.com/baz.tar.gz", - \\ .hash = "1220f1b680b6065fcfc94fe777f22e73bcb7e2767e5f4d99d4255fe76ded69c7a35f", + \\ .hash = "libmp3lame-3.100.1-6-67wlF_KvEwDRCT3pTpcDzi5KGntWCEoM-WtvVPEWdlk5", \\ }, \\ }, \\} @@ -664,7 +662,7 @@ test "basic" { manifest.dependencies.values()[0].location.url, ); try testing.expectEqualStrings( - "1220f1b680b6065fcfc94fe777f22e73bcb7e2767e5f4d99d4255fe76ded69c7a35f", + "libmp3lame-3.100.1-6-67wlF_KvEwDRCT3pTpcDzi5KGntWCEoM-WtvVPEWdlk5", manifest.dependencies.values()[0].hash orelse return error.TestFailed, );