diff options
author | 2023-09-05 14:25:19 -0700 | |
---|---|---|
committer | 2023-09-05 14:25:19 -0700 | |
commit | 1e998c1bf2e0c95fb182eb01806bf11eebe6fed3 (patch) | |
tree | 21ba4289dd7be6b621505ca084960ae6b7c9a5ef /src | |
parent | bc2b55fdeee3001b07252d0f250671ea1876a3ed (diff) | |
download | bun-1e998c1bf2e0c95fb182eb01806bf11eebe6fed3.tar.gz bun-1e998c1bf2e0c95fb182eb01806bf11eebe6fed3.tar.zst bun-1e998c1bf2e0c95fb182eb01806bf11eebe6fed3.zip |
fix(install): ensure all lockfile structs do not have undefined padding (#4401)
* padding sucks
* this assertion is already done elsewhere
* remove test. will be covered alex's pr i believe?
* fix webkit submodule
* fix uws submodule
Diffstat (limited to 'src')
-rw-r--r-- | src/install/bin.zig | 2 | ||||
-rw-r--r-- | src/install/lockfile.zig | 18 | ||||
-rw-r--r-- | src/install/padding_checker.zig | 93 | ||||
-rw-r--r-- | src/install/resolution.zig | 1 | ||||
-rw-r--r-- | src/install/semver.zig | 1 |
5 files changed, 113 insertions, 2 deletions
diff --git a/src/install/bin.zig b/src/install/bin.zig index b559f4f53..0a8d62c8c 100644 --- a/src/install/bin.zig +++ b/src/install/bin.zig @@ -18,6 +18,8 @@ const bun = @import("root").bun; /// - map where keys are names of the binaries and values are file paths to the binaries pub const Bin = extern struct { tag: Tag = Tag.none, + _padding_tag: [3]u8 = .{0} ** 3, + value: Value = Value{ .none = {} }, pub fn verify(this: *const Bin, extern_strings: []const ExternalString) void { diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index 1291c648a..0b5e0d7bc 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -88,6 +88,8 @@ const NameHashMap = std.ArrayHashMapUnmanaged(u32, String, ArrayIdentityContext, const NameHashSet = std.ArrayHashMapUnmanaged(u32, void, ArrayIdentityContext, false); const VersionHashMap = std.ArrayHashMapUnmanaged(u32, Semver.Version, ArrayIdentityContext, false); +const assertNoUninitializedPadding = @import("./padding_checker.zig").assertNoUninitializedPadding; + // Serialized data /// The version of the lockfile format, intended to prevent data corruption for format changes. format: FormatVersion = .v1, @@ -3632,9 +3634,13 @@ pub const Package = extern struct { pub const Meta = extern struct { origin: Origin = Origin.npm, + _padding_origin: u8 = 0, + arch: Npm.Architecture = Npm.Architecture.all, os: Npm.OperatingSystem = Npm.OperatingSystem.all, + _padding_os: u16 = 0, + id: PackageID = invalid_package_id, man_dir: String = String{}, @@ -3731,7 +3737,10 @@ pub const Package = extern struct { var sliced = list.slice(); inline for (FieldsEnum.fields) |field| { - try writer.writeAll(std.mem.sliceAsBytes(sliced.items(@field(Lockfile.Package.List.Field, field.name)))); + const value = sliced.items(@field(Lockfile.Package.List.Field, field.name)); + + comptime assertNoUninitializedPadding(@TypeOf(value)); + try writer.writeAll(std.mem.sliceAsBytes(value)); } const really_end_at = try stream.getPos(); @@ -3782,7 +3791,10 @@ pub const Package = extern struct { var sliced = list.slice(); inline for (FieldsEnum.fields) |field| { - var bytes = std.mem.sliceAsBytes(sliced.items(@field(Lockfile.Package.List.Field, field.name))); + const value = sliced.items(@field(Lockfile.Package.List.Field, field.name)); + + comptime assertNoUninitializedPadding(@TypeOf(value)); + var bytes = std.mem.sliceAsBytes(value); const end_pos = stream.pos + bytes.len; if (end_pos <= end_at) { @memcpy(bytes, stream.buffer[stream.pos..][0..bytes.len]); @@ -3913,7 +3925,9 @@ const Buffers = struct { } pub fn writeArray(comptime StreamType: type, stream: StreamType, comptime Writer: type, writer: Writer, comptime ArrayList: type, array: ArrayList) !void { + comptime assertNoUninitializedPadding(@TypeOf(array)); const bytes = std.mem.sliceAsBytes(array); + const start_pos = try stream.getPos(); try writer.writeIntLittle(u64, 0xDEADBEEF); try writer.writeIntLittle(u64, 0xDEADBEEF); diff --git a/src/install/padding_checker.zig b/src/install/padding_checker.zig new file mode 100644 index 000000000..1d9405a43 --- /dev/null +++ b/src/install/padding_checker.zig @@ -0,0 +1,93 @@ +const std = @import("std"); + +/// In some parts of lockfile serialization, Bun will use `std.mem.sliceAsBytes` to convert a struct into raw +/// bytes to write. This makes lockfile serialization/deserialization much simpler/faster, at the cost of not +/// having any pointers within these structs. +/// +/// One major caveat of this is that if any of these structs have uninitialized memory, then that can leak +/// garbage memory into the lockfile. See https://github.com/oven-sh/bun/issues/4319 +/// +/// The obvious way to introduce undefined memory into a struct is via `.field = undefined`, but a much more +/// subtle way is to have implicit padding in an extern struct. For example: +/// ```zig +/// const Demo = struct { +/// a: u8, // @sizeOf(Demo, "a") == 1, @offsetOf(Demo, "a") == 0 +/// b: u64, // @sizeOf(Demo, "b") == 8, @offsetOf(Demo, "b") == 8 +/// } +/// ``` +/// +/// `a` is only one byte long, but due to the alignment of `b`, there is 7 bytes of padding between `a` and `b`, +/// which is considered *undefined memory*. +/// +/// The solution is to have it explicitly initialized to zero bytes, like: +/// ```zig +/// const Demo = struct { +/// a: u8, +/// _padding: [7]u8 = .{0} ** 7, +/// b: u64, // same offset as before +/// } +/// ``` +/// +/// There is one other way to introduce undefined memory into a struct, which this does not check for, and that is +/// a union with unequal size fields. +pub fn assertNoUninitializedPadding(comptime T: type) void { + const info_ = @typeInfo(T); + const info = switch (info_) { + .Struct => info_.Struct, + .Union => info_.Union, + .Array => |a| { + assertNoUninitializedPadding(a.child); + return; + }, + .Optional => |a| { + assertNoUninitializedPadding(a.child); + return; + }, + .Pointer => |ptr| { + // Pointers aren't allowed, but this just makes the assertion easier to invoke. + assertNoUninitializedPadding(ptr.child); + return; + }, + else => { + return; + }, + }; + // if (info.layout != .Extern) { + // @compileError("assertNoUninitializedPadding(" ++ @typeName(T) ++ ") expects an extern struct type, got a struct of layout '" ++ @tagName(info.layout) ++ "'"); + // } + var i = 0; + for (info.fields) |field| { + const fieldInfo = @typeInfo(field.type); + switch (fieldInfo) { + .Struct => assertNoUninitializedPadding(field.type), + .Union => assertNoUninitializedPadding(field.type), + .Array => |a| assertNoUninitializedPadding(a.child), + .Optional => |a| assertNoUninitializedPadding(a.child), + .Pointer => { + @compileError("Expected no pointer types in " ++ @typeName(T) ++ ", found field '" ++ field.name ++ "' of type '" ++ @typeName(field.type) ++ "'"); + }, + else => {}, + } + } + if (info_ == .Union) { + return; + } + for (info.fields, 0..) |field, j| { + const offset = @offsetOf(T, field.name); + if (offset != i) { + @compileError(std.fmt.comptimePrint( + \\Expected no possibly uninitialized bytes of memory in '{s}', but found a {d} byte gap between fields '{s}' and '{s}' This can be fixed by adding a padding field to the struct like `padding: [{d}]u8 = .{{0}} ** {d},` between these fields. For more information, look at `padding_checker.zig` + , + .{ + @typeName(T), + offset - i, + info.fields[j - 1].name, + field.name, + offset - i, + offset - i, + }, + )); + } + i = offset + @sizeOf(field.type); + } +} diff --git a/src/install/resolution.zig b/src/install/resolution.zig index b1adb3d80..38f238bd4 100644 --- a/src/install/resolution.zig +++ b/src/install/resolution.zig @@ -11,6 +11,7 @@ const VersionedURL = @import("./versioned_url.zig").VersionedURL; pub const Resolution = extern struct { tag: Tag = .uninitialized, + _padding: [7]u8 = .{0} ** 7, value: Value = .{ .uninitialized = {} }, pub fn order( diff --git a/src/install/semver.zig b/src/install/semver.zig index e773f005a..f76238fa5 100644 --- a/src/install/semver.zig +++ b/src/install/semver.zig @@ -602,6 +602,7 @@ pub const Version = extern struct { major: u32 = 0, minor: u32 = 0, patch: u32 = 0, + _tag_padding: [4]u8 = .{0} ** 4, // [see padding_checker.zig] tag: Tag = .{}, // raw: RawType = RawType{}, |