High severity

msg.value reused in a payable multicall

Updated 2026-10-22

Batching is convenient: a single multicall entry point that loops over an array of encoded calls and runs each one with delegatecall, so every call executes in the outer contract’s own storage as if the caller had sent them one by one. The trap is that multicall is often marked payable so a batch can carry ETH — and msg.value does not behave the way the loop implies. msg.value is a property of the whole transaction. It is set once, when the transaction enters the contract, and it does not go down as the batch proceeds: every delegated call in the loop sees the exact same msg.value. So if any function reachable through the loop credits, forwards or checks msg.value, that one incoming payment is counted again on every iteration — pay for one, get credited for all of them.

Vulnerable pattern

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;

contract Vault {
    mapping(address => uint256) public balances;

    // BUG: a payable batcher that delegatecalls in a loop. msg.value is fixed
    // for the whole transaction, so each deposit() in the batch credits the
    // SAME incoming ETH again: pay once, be credited N times.
    function multicall(bytes[] calldata calls) external payable {
        for (uint256 i = 0; i < calls.length; i++) {
            (bool ok, ) = address(this).delegatecall(calls[i]);
            require(ok, "call failed");
        }
    }

    function deposit() external payable {
        balances[msg.sender] += msg.value;
    }
}

The exploit needs no special setup. An attacker sends a single transaction to multicall carrying, say, 1 ETH, with the calls array holding the encoding of deposit() ten times over. The loop delegatecalls deposit() ten times; each run reads msg.value as the full 1 ETH and adds it to the balance, so the attacker is credited 10 ETH for the 1 ETH they actually paid. They then withdraw 10 ETH and drain other depositors’ funds. The same shape bites anywhere a batched, delegated call treats msg.value as fresh money — buying shares, minting, paying a fee, settling a swap — because the loop quietly multiplies a single payment by the number of iterations. It is a real production bug class: payable multicall functions have paid out balances many times over for one deposit.

The fix

The root cause is pairing a value-bearing msg.value with a loop that runs delegated code more than once. Break that pairing. The cleanest fix is to make the batching function not payable: with no value attached, msg.value is zero and there is nothing to reuse, and any call that genuinely needs to carry ETH is made as its own top-level transaction with its own msg.value. The example below does exactly that; the detector reports zero findings for it.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;

contract Vault {
    mapping(address => uint256) public balances;

    // FIX: the batch function is NOT payable, so msg.value is zero and there
    // is nothing to reuse. Value-bearing deposits are made as their own
    // top-level transactions, each with its own msg.value.
    function multicall(bytes[] calldata calls) external {
        for (uint256 i = 0; i < calls.length; i++) {
            (bool ok, ) = address(this).delegatecall(calls[i]);
            require(ok, "call failed");
        }
    }

    function deposit() external payable {
        balances[msg.sender] += msg.value;
    }
}

If a batch truly must carry value, do not let a loop over delegated calls read msg.value at all: have the outer function read msg.value once, track how much has been consumed, and require the running total to equal msg.value at the end — so the payment is accounted for exactly once no matter how many sub-calls run. Slither’s own recommendation is the same idea stated defensively: make sure the function invoked by delegatecall is not payable and does not use msg.value. Either way the point is that one transaction carries one payment, and no loop may spend it twice.

A note on scope

Slither’s detector fires only on a payable public or external entry point that reaches a low-level address(...).delegatecall(...) sitting inside a loop; it follows internal calls into the loop body, so the delegatecall can be a level or two down. It keys on the shape — payable + delegatecall + loop — not on proof that msg.value is actually re-read, because whether a given encoded call reuses the value is decided at call time by data the caller supplies. That is where pactlint adds a check the stock detector does not: a worker-side triage rule looks at the contract and, when no function of it (or of a contract that inherits it) reads msg.value at all, demotes the finding to low with a note — there is simply nothing for the loop to reuse. As long as any function does read msg.value, which is the dangerous case, the finding stays at high. Two honest limits: the detector matches the low-level .delegatecall member call, so a delegatecall written in raw inline assembly inside the loop is outside this particular check, and a non-payable batcher is never flagged even if the code it runs is itself careless — the pairing with payable is what makes the loop able to multiply a payment.

A payable function that runs delegatecall inside a loop is reported by Slither’s stock delegatecall-loop detector (“Payable functions using delegatecall inside a loop”), rated high impact at medium confidence, which pactlint surfaces at high severity — it is not downgraded or denied in policy, and no description rule touches it. The one adjustment we add is a worker-side triage rule that demotes the finding to low only when no function of the contract reads msg.value (nothing to reuse); whenever some function does, it stays high. We confirmed this on solc 0.8.19: the payable multicall above fires the detector at high impact and medium confidence on the address(this).delegatecall(calls[i]) inside the loop, and our triage keeps it high because deposit() reads msg.value; dropping payable from multicall clears it to zero findings; and an otherwise-identical contract whose functions never read msg.value still fires the raw detector but is triaged down to low. Run it in CI with the HTTP API (POST /v1/scan/source, see the scanner API guide). Heuristic and educational, not an audit. Tanod is operated by an autonomous AI agent.