Livepeer Onchain Treasury Upgrade - HChang26's results

Lines of code

https://github.com/code-423n4/2023-08-livepeer/blob/main/contracts/bonding/BondingManager.sol#L273 https://github.com/code-423n4/2023-08-livepeer/blob/main/contracts/bonding/BondingManager.sol#L130 https://github.com/code-423n4/2023-08-livepeer/blob/main/contracts/bonding/BondingManager.sol#L1667 https://github.com/code-423n4/2023-08-livepeer/blob/main/contracts/bonding/BondingManager.sol#L1500

Vulnerability details

Impact

withdrawFees() does not properly update msg.sender check point.

Proof of Concept

Let's take a look at withdrawFees(). autoClaimEarnings() modifier will claim all available earnings before making withdraw.

    function withdrawFees(address payable _recipient, uint256 _amount)
        external
        whenSystemNotPaused
        currentRoundInitialized
        autoClaimEarnings(msg.sender)
    {
        require(_recipient != address(0), "invalid recipient");
        uint256 fees = delegators[msg.sender].fees;
        require(fees >= _amount, "insufficient fees to withdraw");
        delegators[msg.sender].fees = fees.sub(_amount);

        // Tell Minter to transfer fees (ETH) to the address
        minter().trustedWithdrawETH(_recipient, _amount);

        emit WithdrawFees(msg.sender, _recipient, _amount);
    }

modifier autoClaimEarnings(address _delegator) {
        _autoClaimEarnings(_delegator);
        _;
    }

_autoClaimEarnings() calls updateDelegatorWithEarnings(), where state changes are made.

function _autoClaimEarnings(address _delegator) internal {
        uint256 currentRound = roundsManager().currentRound();
        uint256 lastClaimRound = delegators[_delegator].lastClaimRound;
        if (lastClaimRound < currentRound) {
            updateDelegatorWithEarnings(_delegator, currentRound, lastClaimRound);
        }
    }

In updateDelegatorWithEarnings(), rewards are bonded by default which means increase in del.bondedAmount yet no check point was created for this state change. This will result in inaccurate voting power.

        del.lastClaimRound = _endRound;
        // Rewards are bonded by default
        del.bondedAmount = currentBondedAmount;
        del.fees = currentFees;

Tools Used

Manual Review

Recommended Mitigation Steps

    function withdrawFees(address payable _recipient, uint256 _amount)
        external
        whenSystemNotPaused
        currentRoundInitialized
        autoClaimEarnings(msg.sender)
+       autoCheckpoint(msg.sender)
    {
        require(_recipient != address(0), "invalid recipient");
        uint256 fees = delegators[msg.sender].fees;
        require(fees >= _amount, "insufficient fees to withdraw");
        delegators[msg.sender].fees = fees.sub(_amount);

        // Tell Minter to transfer fees (ETH) to the address
        minter().trustedWithdrawETH(_recipient, _amount);

        emit WithdrawFees(msg.sender, _recipient, _amount);
    }

Assessed type

Context