Lines of code
<https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Treasury.sol#L402-L410&gt;

Vulnerability details

Impact

The potential issue identified in the Treasury.rebalanceTreasury()involves the risk of failing to transfer treasury rewards from ETHFromServices to ETHOwned due to insufficient funds in ETHFromServices. This situation can arise when the withdrawToAccount function, called by the dispenser contract, excessively depletes the ETHFromServices balance, leaving an insufficient amount for the annual reward rebalancing.

This can disrupt the intended economic mechanisms of the contract, potentially affecting the treasury’s ability to accumulate and manage its rewards effectively. If left unaddressed, this could lead to financial discrepancies, undermining the contract’s economic model.

Proof of Concept

Here’s a typical scenario:

1. rebalanceTreasury() is called by Tokenomics.checkpoint() typically once a year:

<https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Tokenomics.sol#L1063&gt;

        if (incentives[1] == 0 || ITreasury(treasury).rebalanceTreasury(incentives[1])) {

2. It transfers a portion of the ETHFromServices (ETH collected from service donations) to ETHOwned (ETH owned by the treasury), which represents the treasury’s share of the rewards.

<https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Treasury.sol#L441-L457&gt;

        if (treasuryRewards > 0) {
            uint256 amountETHFromServices = ETHFromServices;
            if (amountETHFromServices >= treasuryRewards) {
                // Update ETH from services value
                amountETHFromServices -= treasuryRewards;
                // Update treasury ETH owned values
                uint256 amountETHOwned = ETHOwned;
                amountETHOwned += treasuryRewards;
                // Assign back to state variables
                ETHOwned = uint96(amountETHOwned);
                ETHFromServices = uint96(amountETHFromServices);
                emit UpdateTreasuryBalances(amountETHOwned, amountETHFromServices);
            } else {
                // There is not enough amount from services to allocate to the treasury
                success = false;
            }
        }

3. If (amountETHFromServices >= treasuryRewards) evaluates to false, the transfer of rewards from ETHFromServices to ETHOwned won’t happen. This situation could arise if there’s an insufficient amount of ETHFromServices compared to the treasuryRewards expected to be rebalanced.
4. This is primarily due to Treasury.withdrawToAccount() that allows the withdrawal of rewards to owners of components / agents. It is called by the dispenser contract and has the potential to reduce ETHFromServices, as it manages payouts to those owner accounts:

<https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Treasury.sol#L402-L409&gt;

        if (accountRewards > 0 && amountETHFromServices >= accountRewards) {
            amountETHFromServices -= accountRewards;
            ETHFromServices = uint96(amountETHFromServices);
            emit Withdraw(ETH_TOKEN_ADDRESS, account, accountRewards);
            (success, ) = account.call{value: accountRewards}("");
            if (!success) {
                revert TransferFailed(address(0), address(this), account, accountRewards);
            }

5. Now, mapEpochTokenomics[eCounter] is epoch specific and will not roll over when to the next eCounter when checkpoint() will be called again in about a year. But even if it did roll over, this would be mean the treasury would be deprived of the supposed rewards for an entire year.

<https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Tokenomics.sol#L906-L915&gt;

        uint256 eCounter = epochCounter;
        TokenomicsPoint storage tp = mapEpochTokenomics[eCounter];

        // 0: total incentives funded with donations in ETH, that are split between:
        // 1: treasuryRewards, 2: componentRewards, 3: agentRewards
        // OLAS inflation is split between:
        // 4: maxBond, 5: component ownerTopUps, 6: agent ownerTopUps
        uint256[] memory incentives = new uint256[](7);
        incentives[0] = tp.epochPoint.totalDonationsETH;
        incentives[1] = (incentives[0] * tp.epochPoint.rewardTreasuryFraction) / 100;

<https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Tokenomics.sol#L1063&gt;

        if (incentives[1] == 0 || ITreasury(treasury).rebalanceTreasury(incentives[1])) {

Tools Used

Manual

Recommended Mitigation Steps

Consider setting a threshold mechanism that could be implemented in the withdrawToAccount function. This threshold would ensure that a certain amount of ETH is always reserved in ETHFromServices for the treasury’s rewards.

• The threshold could be an estimated or projected amount, calculated based on expected treasury rewards.
• Before executing any withdrawal in the withdrawToAccount function, the contract would check whether the withdrawal would cause ETHFromServices to drop below this threshold.
• If the threshold would be breached, the withdrawal could be limited or halted to maintain the necessary balance. (Note: This would exacerbate the issue I have reported in a different submission concerning unclaimable rewards due to the first attempt failure.)
• Where possible, the threshold value would need to be dynamically calculated or periodically updated to reflect the current or projected treasuryRewards.

Assessed type

Timing

---

The text was updated successfully, but these errors were encountered:

All reactions