Olas - c0pp3rscr3w3r's results

Olas is a unified network for off-chain services like automation, oracles, co-owned AI. It offers a stack for building services and a protocol for incentivizing their creation and their operation in a co-owned and decentralized way.

General Information

Platform: Code4rena

Start Date: 21/12/2023

Pot Size: $90,500 USDC

Total HM: 10

Participants: 39

Period: 18 days

Judge: LSDan

Total Solo HM: 5

Id: 315

League: ETH

Olas

Findings Distribution

Researcher Performance

Rank: 7/39

Findings: 1

Award: $830.39

🌟 Selected for report: 0

πŸš€ Solo Findings: 0

Findings Information

🌟 Selected for report: hash

Also found by: 0xTheC0der, HChang26, c0pp3rscr3w3r

Labels

bug
3 (High Risk)
disagree with severity
partial-50
sponsor acknowledged
sufficient quality report
upgraded by judge
duplicate-373

Awards

830.3941 USDC - $830.39

External Links

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

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

Assessed type

Invalid Validation

#0 - c4-pre-sort

2024-01-10T15:06:50Z

alex-ppg marked the issue as primary issue

#1 - c4-pre-sort

2024-01-10T15:06:54Z

alex-ppg marked the issue as sufficient quality report

#2 - c4-sponsor

2024-01-16T15:28:16Z

kupermind marked the issue as disagree with severity

#3 - c4-sponsor

2024-01-16T15:28:26Z

kupermind (sponsor) acknowledged

#4 - c4-judge

2024-01-20T13:01:06Z

dmvt marked the issue as selected for report

#5 - kupermind

2024-01-22T18:12:55Z

@c4-judge This issue relates to the issue #373, but it does not specify what is the exact issue that car arise. We acknowledge the issue, however this deserves a much smaller attention compared to #373.

#6 - c4-judge

2024-01-23T12:44:31Z

dmvt marked the issue as duplicate of #373

#7 - c4-judge

2024-01-23T12:45:30Z

dmvt changed the severity to 3 (High Risk)

#8 - c4-judge

2024-01-23T12:45:40Z

dmvt marked the issue as partial-50

#9 - dmvt

2024-01-23T12:47:49Z

I agree with the sponsor on this one. The two issues are related and addressing the highest quality one addresses this. I've marked these as 50% due to the fact that as written they are medium, not high.

#10 - c4-judge

2024-01-23T21:11:26Z

dmvt marked the issue as not selected for report

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter