Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 6 additions & 6 deletions bindings/napi/blst.zig
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ pub const PublicKey = struct {
pub fn toBytes(self: *const PublicKey, compress: ?js.Boolean) !js.Uint8Array {
if (try boolOrDefault(compress, true)) {
const bytes = self.raw.compress();
return js.Uint8Array.from(bytes[0..]);
return js.Uint8Array.fromExternal(bytes[0..]);
}
const bytes = self.raw.serialize();
return js.Uint8Array.from(bytes[0..]);
return js.Uint8Array.fromExternal(bytes[0..]);
Comment on lines 137 to +142

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This implementation introduces a critical use-after-free vulnerability.

The self.raw.compress() and self.raw.serialize() functions return arrays that are stored on the current function's stack. The slice passed to js.Uint8Array.fromExternal then points to this stack memory.

When toBytes returns, its stack frame is deallocated, making the pointer held by the JavaScript Uint8Array invalid. Any subsequent access to this Uint8Array in JS will read from deallocated memory, leading to undefined behavior, memory corruption, or a crash.

To fix this, you must allocate the buffer on the heap. The fromExternal function can then wrap this heap-allocated buffer, and its associated finalizer (which I assume zapi sets up) will be responsible for freeing the memory.

Here is a suggested implementation that correctly allocates memory on the heap:

        const do_compress = try boolOrDefault(compress, true);
        const len = if (do_compress) NativePublicKey.COMPRESS_SIZE else NativePublicKey.SERIALIZE_SIZE;

        const heap_bytes = try allocator.alloc(u8, len);
        // This errdefer is important in case fromExternal throws.
        errdefer allocator.free(heap_bytes);

        if (do_compress) {
            const stack_bytes = self.raw.compress();
            @memcpy(heap_bytes, stack_bytes[0..]);
        } else {
            const stack_bytes = self.raw.serialize();
            @memcpy(heap_bytes, stack_bytes[0..]);
        }

        return js.Uint8Array.fromExternal(heap_bytes);

}

pub fn toHex(self: *const PublicKey, compress: ?js.Boolean) !js.String {
Expand Down Expand Up @@ -214,10 +214,10 @@ pub const Signature = struct {
pub fn toBytes(self: *const Signature, compress: ?js.Boolean) !js.Uint8Array {
if (try boolOrDefault(compress, true)) {
const bytes = self.raw.compress();
return js.Uint8Array.from(bytes[0..]);
return js.Uint8Array.fromExternal(bytes[0..]);
}
const bytes = self.raw.serialize();
return js.Uint8Array.from(bytes[0..]);
return js.Uint8Array.fromExternal(bytes[0..]);
Comment on lines 215 to +220

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Similar to PublicKey.toBytes, this function has a use-after-free vulnerability because it passes a slice of stack-allocated memory to js.Uint8Array.fromExternal. The memory must be allocated on the heap to prevent the JavaScript Uint8Array from pointing to invalid memory after this function returns.

        const do_compress = try boolOrDefault(compress, true);
        const len = if (do_compress) NativeSignature.COMPRESS_SIZE else NativeSignature.SERIALIZE_SIZE;

        const heap_bytes = try allocator.alloc(u8, len);
        // This errdefer is important in case fromExternal throws.
        errdefer allocator.free(heap_bytes);

        if (do_compress) {
            const stack_bytes = self.raw.compress();
            @memcpy(heap_bytes, stack_bytes[0..]);
        } else {
            const stack_bytes = self.raw.serialize();
            @memcpy(heap_bytes, stack_bytes[0..]);
        }

        return js.Uint8Array.fromExternal(heap_bytes);

}

pub fn toHex(self: *const Signature, compress: ?js.Boolean) !js.String {
Expand Down Expand Up @@ -295,9 +295,9 @@ pub const SecretKey = struct {
}

/// Serializes the SecretKey to bytes (32 bytes).
pub fn toBytes(self: *const SecretKey) js.Uint8Array {
pub fn toBytes(self: *const SecretKey) !js.Uint8Array {
const bytes = self.raw.serialize();
return js.Uint8Array.from(bytes[0..]);
return js.Uint8Array.fromExternal(bytes[0..]);
Comment on lines 299 to +300

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This function also has a use-after-free vulnerability. The serialize() method returns a stack-allocated array, and a slice of it is passed to fromExternal. This memory will be invalid after the function returns. You need to copy the bytes to a heap-allocated buffer.

        const stack_bytes = self.raw.serialize();
        const heap_bytes = try allocator.alloc(u8, stack_bytes.len);
        errdefer allocator.free(heap_bytes);
        @memcpy(heap_bytes, stack_bytes[0..]);
        return js.Uint8Array.fromExternal(heap_bytes);

}

pub fn toHex(self: *const SecretKey) !js.String {
Expand Down
4 changes: 2 additions & 2 deletions build.zig.zon
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
.hash = "zig_yaml-0.1.0-C1161kFWAwDxjKAFmklKwWVDvz2mmq0Q__bDhGGjeyd3",
},
.zapi = .{
.url = "git+https://github.com/chainsafe/zapi?ref=zapi-v2.0.0#f9fa8b0237352326e9f970b62588b6af01e3e384",
.hash = "zapi-2.0.0-rIqzUcc3BABKRuJlzpYNtiMVCOFybuVSGOLmk0KOCeel",
.url = "git+https://github.com/chainsafe/zapi?ref=zapi-v2.1.0#c5c877af9742d9d7fd6cab2ce6fec698817d56cd",
.hash = "zapi-2.1.0-rIqzUbxNBADOW16nSYQfUOtib1TgC8PxbR4ggN6ezYfA",
},
.zbench = .{
.url = "git+https://github.com/hendriknielaender/zBench#b2b89c475e3ef1bb2bd71255c80478a82d3e0ca8",
Expand Down
Loading