bunker.finance contest - 0x1f8b's results

Lines of code

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/PriceOracleImplementation.sol#L29 https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/PriceOracleImplementation.sol#L31 https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/PriceOracleImplementation.sol#L37

Vulnerability details

Impact

According to Chainlink's documentation, the latestAnswer function is deprecated.

Proof of Concept

This function does not error if no answer has been reached but returns 0. Besides, the latestAnswer is reported with 18 decimals for crypto quotes but 8 decimals for FX quotes (See Chainlink FAQ for more details).

Also there are some cases when 0 is returned, this could produce a wrong computation in prices because the price never will be zero, otherwise it should be aborted.

Recommended Mitigation Steps

Use the latestRoundData function to get the price instead. Add checks on the return data with proper revert messages if the price is stale or the round is uncomplete, for example:

(uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = ChainlinkFeed(0x986b5E1e1755e3C2440e960477f25201B0a8bbD4).latestRoundData();
require(answeredInRound >= roundID, "...");
require(timeStamp != 0, "...");

1. Code style:

• Remove the first _ from public methods CErc20.sol#L146
• Typo in checck at PriceOracleImplementation.sol#L34

2. The method CNftPriceOracle.addAddressMapping is not possible to be reverted, so if any admin set a wrong address for a specific underlying, it won't be possible to be fixed.

1. There are require messages bigger than 32 bytes. More than 32 bytes for message will incur an extra gas costs.

• CNftPriceOracle.sol#L64
• CNftPriceOracle.sol#L70
• CNftPriceOracle.sol#L90
• UniswapV2PriceOracle.sol#L68
• UniswapV2PriceOracle.sol#L92
• UniswapV2PriceOracle.sol#L116
• UniswapV2PriceOracle.sol#L128
• UniswapV2PriceOracle.sol#L131
• UniswapV2PriceOracle.sol#L137
• UniswapV2PriceOracle.sol#L149
• UniswapV2PriceOracle.sol#L152
• CNft.sol#L24-L25
• CNft.sol#L40
• CNft.sol#L52
• CNft.sol#L69
• CNft.sol#L93
• CNft.sol#L100
• CNft.sol#L124
• CNft.sol#L148
• CNft.sol#L204
• CNft.sol#L208-L209
• CNft.sol#L279

2. Use 1 ether instead of 10**18

• CNftPriceOracle.sol#L99