Backd contest - 0x1f8b's results

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/oracles/ChainlinkOracleProvider.sol#L55

Vulnerability details

Vulnerability

On ChainlinkOracleProvider.sol#L55, we are using latestRoundData, but there are no validations that the data is not stale.

The current code is:

(, int256 answer, , uint256 updatedAt, ) = AggregatorV2V3Interface(feed).latestRoundData();

But is missing the checks to validate the data is stale

(uint80 round, int256 answer, , uint256 updatedAt, uint80 answeredInRound) = AggregatorV2V3Interface(feed).latestRoundData();
require(answeredInRound >= round, "Stale price");

This could affect in all the logic, including funds.

1. Change the incremental logic from i++ to ++i in order to save some opcodes:

• RoleManager.sol#L80
• CompoundHandler.sol#L135
• CTokenRegistry.sol#L61
• TopUpAction.sol#L188
• TopUpAction.sol#L456
• TopUpAction.sol#L479
• TopUpAction.sol#L506
• TopUpAction.sol#L891
• TopUpKeeperHelper.sol#L43-L46
• TopUpKeeperHelper.sol#L72
• TopUpKeeperHelper.sol#L93
• TopUpKeeperHelper.sol#L165
• Controller.sol#L117
• BkdLocker.sol#L310

2. Use delete instead of set to default value (false or 0)

• RoleManager.sol#L156
• TopUpActionFeeHandler.sol#L140

3. Cache static keccak256 in order to reduce the computation.

• keccak256(abi.encodePacked("cETH") at CTokenRegistry.sol#L67

4. Remove returns because always returns the same value.

• LpToken.sol#L38
• CvxCrvRewardsLocker.sol#L89
• CvxCrvRewardsLocker.sol#L105
• CvxCrvRewardsLocker.sol#L144
• CvxCrvRewardsLocker.sol#L153
• CvxCrvRewardsLocker.sol#L163
• CvxCrvRewardsLocker.sol#L214
• CvxCrvRewardsLocker.sol#L224