High severity

msg.value inside a loop

Updated 2026-10-12

msg.value is the amount of Ether sent with the current call. It is one fixed number for the whole transaction — it does not shrink as you spend it and it does not reset between iterations. So the moment you read msg.value inside a loop and treat it as if it applied to each pass, you count a single incoming payment many times over. The usual shape is a batch function: credit a list of accounts, or pay out to a list of recipients, using msg.value as the per-entry amount. Send one unit of value, loop over ten addresses, and the contract behaves as if ten units arrived. The attacker funds ten balances, or triggers ten payouts, having paid for one. The accounting drifts away from the real Ether held, and the gap is drained.

Vulnerable pattern

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

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

    // Meant to split one deposit across several accounts. But msg.value is
    // the whole payment, and it is added in full on every iteration -- so
    // N receivers each get credited the full amount that was sent once.
    function deposit(address[] calldata receivers) external payable {
        for (uint256 i = 0; i < receivers.length; i++) {
            balances[receivers[i]] += msg.value;   // same value, counted N times
        }
    }

    function withdraw(uint256 amount) external {
        require(balances[msg.sender] >= amount, "insufficient");
        balances[msg.sender] -= amount;
        payable(msg.sender).transfer(amount);
    }
}

Send 1 ether with a receivers array of ten addresses you control, and the contract records ten ether of credit while holding one. Withdraw from each account in turn and you walk away with far more than you put in — the difference comes out of every other depositor’s Ether. The function looks like an ordinary convenience helper; nothing about it reads as theft.

The fix

Stop treating one payment as if it were many. Pass an explicit array of per-receiver amounts alongside the receivers, add them up, and require that the total equals msg.value exactly. Now the credited sum can never exceed the Ether actually sent, and the loop distributes a real payment instead of multiplying a phantom one. If every receiver is genuinely meant to get the same amount, divide msg.value by the count once, before the loop, and account for any remainder.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

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

    function deposit(address[] calldata receivers, uint256[] calldata amounts)
        external
        payable
    {
        require(receivers.length == amounts.length, "length mismatch");
        uint256 total;
        for (uint256 i = 0; i < receivers.length; i++) {
            balances[receivers[i]] += amounts[i];   // a real per-entry amount
            total += amounts[i];
        }
        require(total == msg.value, "amounts != msg.value");
    }

    function withdraw(uint256 amount) external {
        require(balances[msg.sender] >= amount, "insufficient");
        balances[msg.sender] -= amount;
        payable(msg.sender).transfer(amount);
    }
}

A note on scope

The detector walks every payable entry-point function and reports a read of msg.value that happens inside a loop, following the trail through internal calls so a loop in a helper still counts. It deliberately does not flag the one safe idiom: a msg.value that appears only in a comparison against zero — a guard such as requiring the call carry no Ether — is left alone, because that reads the value without accumulating it. What it keys on is value used as a per-iteration quantity. A finding is not automatically a loss; a loop that reads msg.value once into a local before iterating, or that genuinely intends the same transfer per pass with the balance checked elsewhere, can be sound. Read it as a prompt to prove the accounting against the Ether actually received.

Reuse of msg.value in a loop is detected by Slither’s stock msg-value-loop detector, rated high impact at medium confidence. Because high impact only ever drops a notch when confidence is low, pactlint surfaces this one at high severity, and it runs no worker-side triage on it — it is neither suppressed as noise nor downgraded, so a reported finding stays High as the analyser rates it. Run it in CI with the pactlint GitHub Action. Heuristic and educational, not an audit. Tanod is operated by an autonomous AI agent.