Olympus DAO contest - itsmeSTYJ's results

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/main/src/policies/Heart.sol#L106

Vulnerability details

Impact

If there are insufficient rewards remaining in the contract, _issueReward() will fail to transfer and thus beat() cannot be called. Again, there is an assumption that there will always be sufficient rewards in the contract but it is prudent to not assume otherwise.

Recommended Mitigation Steps

Transfer the minimum between the amount of reward to be paid and the token balance of the contract i.e.

function min(uint256 a, uint256 b) internal returns (uint256) {
    if(a < b) return a;
    return b;
}

function _issueReward(address to_) internal {
    uint256 balance = rewardToken.balanceOf(address(this));
    uint256 amt = min(balance, reward);
    rewardToken.safeTransfer(to_, amt);
    emit RewardIssued(to_, amt);
}

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/main/src/modules/RANGE.sol#L97-L98

Vulnerability details

Impact

If cushion spread is > wall spread, the entire contracts are bricked because it operates on the assumption that the cushion spread is strictly less than wall spread. Do note that this is only a problem in the constructor i.e. the setSpreads() function contains this validation check.

Recommended Mitigation Steps

Require that the cushion spread is < wall spread.

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/main/src/modules/PRICE.sol#L167 https://github.com/code-423n4/2022-08-olympus/blob/main/src/modules/PRICE.sol#L173

Vulnerability details

Impact

It is possible that an incorrect value for currentPrice is used when latestRoundData() is called because no validation is done on the roundId and that price cannot be 0. Do note that the impact is somewhat minimized because a simple moving average is used but it is still prudent to add these validation checks to ensure that the protocol reflects the most up-to-date and correct price for OHMv2 and the reserve token.

Recommended Mitigation Steps

check that the roundId is increasing and that price is > 0.

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/main/src/policies/Heart.sol#L103

Vulnerability details

Impact

It is possible to call beat() multiple times consecutively because lastBeat is not correctly tracked.

Proof of Concept

Assume the current timestamp is 1000 and the observation frequency is 100. beat() is called once and lastBeat is updated to 1100 which is correct. Assume that 323 seconds have passed i.e. the current time is now 1423. It is now possible to call beat() another 4 more times since lastBeat is always incremented by observation frequency.

Recommended Mitigation Steps

Replace lastBeat += frequency(); with lastBeat = block.timestamp + frequency();

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/main/src/modules/PRICE.sol#L32-L33

Vulnerability details

Impact

Should anything happen to chainlink oracles e.g. pausing of price feed, error in configuration or even when gas prices are too high, the protocol’s functionality would be halted as there are no fallback oracles to rely on.

Recommended Mitigation Steps

• Introduce a fallback oracle and periodically ensure that fallback oracle is working.
• Introduce pausing functionality as a last resort.