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
Rank: 7/39
Findings: 1
Award: $830.39
π Selected for report: 0
π Solo Findings: 0
π Selected for report: hash
Also found by: 0xTheC0der, HChang26, c0pp3rscr3w3r
830.3941 USDC - $830.39
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
The service Owner loses all of his topUp savings in Olas when the inflation limit is hit
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
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); . . }
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.
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); } }
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
Manual analysis
provide proper checks for Olas mint, either revert if supply is unavailable or provide better checks in Treasury
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