[A23-1] [HIGH] LibFlow.stackToFlow: malformed-stack revert paths uncovered

[thedavidmeister]

Gap

LibFlow.stackToFlow (src/lib/LibFlow.sol:45-68) calls consumeSentinelTuples three times, each of which reverts with MissingSentinel(RAIN_FLOW_SENTINEL) when the sentinel is absent from the remaining stack. This revert path is the only thing that protects Flow.flow from interpreting malformed evaluator output as a valid FlowTransferV1. The path is completely uncovered.

There is no test that:

1. Calls stackToFlow (or flow.flow(...)) with an empty stack new uint256[](0).
2. Calls stackToFlow with a stack missing one or more of the three sentinels.
3. Calls stackToFlow with a stack where the items between sentinels are not a multiple of 4 (erc20/erc721) or 5 (erc1155) — the cursor in consumeSentinelTuples walks n words at a time, so a count off by a non-multiple either underflows or skips past the sentinel.
4. Places a value equal to RAIN_FLOW_SENTINEL inside a tuple field (e.g., erc20[0].amount == RAIN_FLOW_SENTINEL). The two-pass implementation in consumeSentinelTuples walks from upper down by size words and stops at the first match — a planted sentinel inside a tuple will be misinterpreted as the section boundary, silently truncating that section and corrupting the next section. Existing preview/transfer tests guard against this with vm.assume(Sentinel.unwrap(RAIN_FLOW_SENTINEL) != <value>), which proves the assumption is necessary but never exercises what happens when it fails.

Source

src/lib/LibFlow.sol:52, 57, 62 (the three consumeSentinelTuples calls)

Existing tests

• test/src/concrete/Flow.preview.t.sol:168 (testFlowBasePreviewEmptyFlowIO) — only-sentinels, zero tuples each. Passes through happy path.
• All other Flow.preview.t.sol tests — well-formed stacks generated by LibStackGeneration.generateFlowStack.
• MissingSentinel is tested in the upstream rain.solmem repo but not in this repo's context where the three-section invariant matters.

Proposed tests

Add to test/src/concrete/Flow.preview.t.sol (or a new Flow.malformedStack.t.sol):

import {MissingSentinel} from "rain.solmem/lib/LibStackSentinel.sol";

function testStackToFlowEmptyStackReverts() external {
    (IFlowV5 flow,) = deployFlow();
    vm.expectRevert(abi.encodeWithSelector(MissingSentinel.selector, RAIN_FLOW_SENTINEL));
    flow.stackToFlow(new uint256[](0));
}

function testStackToFlowMissingERC1155SentinelReverts() external {
    (IFlowV5 flow,) = deployFlow();
    // Only erc20 and erc721 sentinels, no erc1155 sentinel at the top.
    uint256[] memory stack = new uint256[](2);
    stack[0] = Sentinel.unwrap(RAIN_FLOW_SENTINEL);
    stack[1] = Sentinel.unwrap(RAIN_FLOW_SENTINEL);
    vm.expectRevert(abi.encodeWithSelector(MissingSentinel.selector, RAIN_FLOW_SENTINEL));
    flow.stackToFlow(stack);
}

function testStackToFlowSentinelInsideTupleTruncates() external {
    // Plant RAIN_FLOW_SENTINEL inside an erc20 tuple's `amount` field.
    // The earlier sentinel scan stops at the planted value, so the section
    // boundary is misread and the next section's parsing corrupts.
    (IFlowV5 flow,) = deployFlow();

    uint256 sentinel = Sentinel.unwrap(RAIN_FLOW_SENTINEL);
    // Layout (bottom -> top):
    //   sentinel (erc1155)
    //   sentinel (erc721)
    //   sentinel (erc20)        <-- the legit erc20 boundary
    //   token, from, to, sentinel  <-- planted tuple
    uint256[] memory stack = new uint256[](7);
    stack[0] = sentinel;
    stack[1] = sentinel;
    stack[2] = sentinel;
    stack[3] = uint256(uint160(address(0xAAAA)));
    stack[4] = uint256(uint160(address(this)));
    stack[5] = uint256(uint160(address(0xBBBB)));
    stack[6] = sentinel; // would be `amount`

    // Document the actual behaviour (revert or silent truncation) and pin it.
    // If silent truncation: assert the resulting FlowTransferV1 has the
    // erc20 array length corresponding to the misread boundary, NOT 1.
    FlowTransferV1 memory ft = flow.stackToFlow(stack);
    // Pin whatever the current behaviour is — the point is that the test
    // exists and a future change is forced to update the assertion.
    assertEq(ft.erc20.length, 0, "erc20 truncated by planted sentinel");
}

function testStackToFlowMalformedTupleCountReverts() external {
    // 3 items between erc20 sentinel and top — not a multiple of 4. The
    // cursor in consumeSentinelTuples walks 4 at a time and either
    // underflows or skips past the sentinel.
    (IFlowV5 flow,) = deployFlow();
    uint256 sentinel = Sentinel.unwrap(RAIN_FLOW_SENTINEL);
    uint256[] memory stack = new uint256[](6);
    stack[0] = sentinel; // erc1155
    stack[1] = sentinel; // erc721
    stack[2] = sentinel; // erc20
    stack[3] = 1;
    stack[4] = 2;
    stack[5] = 3;
    vm.expectRevert(abi.encodeWithSelector(MissingSentinel.selector, RAIN_FLOW_SENTINEL));
    flow.stackToFlow(stack);
}

Each test must mutation-test: temporarily change consumeSentinelTuples arity or remove the MissingSentinel revert and confirm the relevant test fails.