Skip to content

Reads stale sol_memcmp_ inputs after result triggered account growth #1742

Open

Description

@caomingpei

Description

Relationship to
Syndica/sig#1741.
Both findings
belong to the translated host pointer lifetime across account growth family
because a later growth-capable translation invalidates an earlier host
reference. However, #1741 loses a write through a stale mutable getReturnData output,
while this finding reads a stale constant memcmp input and has a different
fix site.

Summary

Sig translates and retains both sol_memcmp_ input slices before translating
the mutable result pointer. If result translation grows a directly mapped
account, the account-backed input becomes stale. Sig then computes the
comparison from the old allocation.

In Sig Debug and production ReleaseSafe, two equal four-byte inputs compare as
153 instead of 0. The transaction succeeds and commits different account
data from Agave. ReleaseFast happens to return the correct value for this
allocation layout, but the retained pointer is invalid in every build.

Root Cause

Sig saves s1 and s2, translates the result, and only then reads the saved
slices:
[memops.zig:95-120].

Result translation can invoke the production account-growth handler and
replace the account backing:
[execute.zig:305-398].
In checked builds, the released first byte becomes 0xaa, so
0xaa - 0x11 = 153.

Agave computes the comparison before translating the mutable result:
[mem_ops.rs:64-109].

Suggested Fix

Compute and retain the i32 comparison while both input slices are valid.
Only then translate and write the result pointer. No input reference should
remain live across the growth-capable mutable translation.

How to Reproduce the Bug

Environment

Sig:          37bb06aae349e51ef733f0d31f2e704dfcb73e57
Agave:        8ce33d59e3e250aedd9461e0a6cb977c2d948d2b
Features:     SPAR + VASA + direct account mapping
Sig profiles: Debug, ReleaseSafe, and ReleaseFast

PoC

Add this case beside the existing memcmp tests in
shared/vm/syscalls/memops.zig. The callback copies the account into its new
backing and poisons the released bytes with 0xaa, matching the deterministic
checked-build result.

test "memcmp reads inputs before result-triggered growth" {
    const allocator = std.testing.allocator;
    const input_addr = memory.INPUT_START;
    const expected_addr = memory.HEAP_START;

    var old: [8]u8 align(8) = .{ 0x11, 0x22, 0x33, 0x44, 0, 0, 0, 0 };
    var new: [12]u8 align(8) = @splat(0);
    const expected = [_]u8{ 0x11, 0x22, 0x33, 0x44 };

    const Grow = struct {
        old: *[8]u8,
        new: *[12]u8,

        fn handle(
            raw: *anyopaque,
            region: *memory.Region,
            _: u64,
            _: memory.MemoryState,
            _: u64,
            _: u64,
        ) void {
            const self: *@This() = @ptrCast(@alignCast(raw));
            @memcpy(self.new[0..self.old.len], self.old);
            @memset(self.old, 0xaa);
            region.host_memory = .{ .mutable = self.new };
            region.vm_addr_end = region.vm_addr_start + self.new.len;
        }
    };
    var grow = Grow{ .old = &old, .new = &new };

    var map = try MemoryMap.init(
        allocator,
        &.{
            memory.Region.init(.constant, &expected, expected_addr),
            memory.Region.init(.mutable, &old, input_addr),
        },
        .v2,
        .{ .aligned_memory_mapping = false, .virtual_address_space_adjustments = true },
    );
    defer map.deinit(allocator);
    map.setAccessViolationHandler(.{ .ctx = &grow, .call = Grow.handle });

    var prng = std.Random.DefaultPrng.init(0);
    var cache, var tc = try sig.runtime.testing.createTransactionContext(
        allocator,
        prng.random(),
        .{ .compute_meter = 10_000 },
    );
    defer {
        sig.runtime.testing.deinitTransactionContext(allocator, &tc);
        cache.deinit(allocator);
    }

    var registers = RegisterMap.initFill(0);
    registers.set(.r1, input_addr);
    registers.set(.r2, expected_addr);
    registers.set(.r3, 4);
    registers.set(.r4, input_addr + 8); // result translation triggers growth
    try memcmp(&tc, &map, &registers);

    const observed = std.mem.readInt(i32, new[8..12], .little);
    try std.testing.expectEqual(@as(i32, 0), observed);
}

Current Sig fails with:

expected 0, found 153

The input occupies [input, input + 4). The disjoint result at input + 8
crosses the original 8-byte region and triggers growth before Sig reads the
saved input pointer. Moving the comparison before result translation makes the
case pass.

Differential Result

Runtime Execution Comparison Final account prefix
Sig Debug success 153 7b 99 00 00
Sig ReleaseSafe success 153 7b 99 00 00
Sig ReleaseFast success 0 42 00 00 00
Agave success 0 42 00 00 00

A 12-byte account avoids growth and makes all builds return 0. Placing both
inputs on the stack also removes the difference.

Additional Context

No response

Activity

  1. 0xzrf commented on Jul 26, 2026

    @0xzrf
    Contributor

    not that important, but it fails on all 3 runtimes for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    • Status
      No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions