code-423n4 / 2022-08-olympus-findings

5 stars 4 forks source link

Potential Reward tokens Rug Pull by heart admin. #14

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Heart.sol#L150

Vulnerability details

The heart_admin of the Heart contract can transfer all rewards tokens to himself. There is no check to prevent that reward token not being withdrawn. Since it is issued as rewards when calling beat() function

Impact

heart_admin can sweep reward token balances of the heart contract.

Recommended Mitigation Steps

Can add this check:

require(_token != rewardToken, "Not Withdrawable!");
Oighty commented 1 year ago

The heart_admin will be a protocol-owned multi-sig. The reason that withdrawUnspentRewards exist is to be able to reclaim tokens in the contract. This is by design.

0xean commented 1 year ago

Downgrading to QA. There is no benefit for a non malicious admin to remove these rewards, presumably they are the one who put the rewards in the contract in the first place and have the ability to change the reward token. So the mitigation also woudlnt work.