c4-bot-6 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/tokenomics/contracts/Dispenser.sol#L98-L105 https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/tokenomics/contracts/Tokenomics.sol#L1144-L1149 https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/tokenomics/contracts/Treasury.sol#L412-L418 https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/governance/contracts/OLAS.sol#L75-L83

Vulnerability details

Impact

The service Owner loses all of his topUp savings in Olas when the inflation limit is hit

Proof of Concept

Assume, there has been donations to the service Id and it has collected some donations and has it stored in

mapUnitIncentives[unitTypes[i]][unitIds[i]].reward
and
mapUnitIncentives[unitTypes[i]][unitIds[i]].topUp

1) User withdraws from claimOwnerIncentives() in Dispenser

function claimOwnerIncentives(uint256[] memory unitTypes, uint256[] memory unitIds) external
        returns (uint256 reward, uint256 topUp){
.
.
        // Calculate incentives
        (reward, topUp) = ITokenomics(tokenomics).accountOwnerIncentives(msg.sender, unitTypes, unitIds);

        bool success;
        // Request treasury to transfer funds to msg.sender if reward > 0 or topUp > 0
        if ((reward + topUp) > 0) {
            success = ITreasury(treasury).withdrawToAccount(msg.sender, reward, topUp);
.
.
        }

2) accountOwnerIncentives in tokenomics resets reward and topUp

function accountOwnerIncentives(address account, uint256[] memory unitTypes, uint256[] memory unitIds) external
        returns (uint256 reward, uint256 topUp)
{
            .
            .
            // Accumulate total rewards and clear their balances
            reward += mapUnitIncentives[unitTypes[i]][unitIds[i]].reward;
->          (mapUnitIncentives[unitTypes[i]][unitIds[i]].reward = 0; 
            // Accumulate total top-ups and clear their balances
            topUp += mapUnitIncentives[unitTypes[i]][unitIds[i]].topUp;
->          mapUnitIncentives[unitTypes[i]][unitIds[i]].topUp = 0; //@audit
      }
}

Here, we can see that reward and topup are set to 0 once the function is called. This is okay if we revert when transaction to service owner fails, but this wasn't done properly for topups.

3) withdrawToAccount in Treasury verifies sending rewards and topups

    function withdrawToAccount(address account, uint256 accountRewards, uint256 accountTopUps) external
        returns (bool success)
    {
    .
    .
        // Send ETH rewards, if any
        if (accountRewards > 0 && amountETHFromServices >= accountRewards) {
            amountETHFromServices -= accountRewards;
            ETHFromServices = uint96(amountETHFromServices);
            emit Withdraw(ETH_TOKEN_ADDRESS, account, accountRewards);
            (success, ) = account.call{value: accountRewards}("");
 ->           if (!success) { 
                //@audit proper validation done in rewards
                revert TransferFailed(address(0), address(this), account, accountRewards);
            }
        }

        // Send OLAS top-ups
        if (accountTopUps > 0) {
            // Tokenomics has already accounted for the account's top-up amount,
            // thus the the mint does not break the inflation schedule
->            IOLAS(olas).mint(account, accountTopUps);
->            success = true;
              //@audit improper validation
            emit Withdraw(olas, account, accountTopUps);
        }
}

4) Olas mints token provided inflationControl is true

    function mint(address account, uint256 amount) external {
        .
        .
        // Check the inflation schedule and mint
        if (inflationControl(amount)) {
            _mint(account, amount);
        }
    }

inflationControl checks if the amount requested can be satisfied with the current supply. In the case its not, mint fails silently while the top up earnings of service owner are nulled

Tools Used

Manual analysis

Recommended Mitigation Steps

provide proper checks for Olas mint, either revert if supply is unavailable or provide better checks in Treasury

Assessed type

Invalid Validation

c4-pre-sort commented 9 months ago

alex-ppg marked the issue as primary issue

c4-pre-sort commented 9 months ago

alex-ppg marked the issue as sufficient quality report

c4-sponsor commented 8 months ago

kupermind marked the issue as disagree with severity

c4-sponsor commented 8 months ago

kupermind (sponsor) acknowledged