Platform: Code4rena
Start Date: 28/01/2022
Pot Size: $30,000 USDC
Total HM: 4
Participants: 22
Period: 3 days
Judge: GalloDaSballo
Total Solo HM: 2
Id: 80
League: ETH
Rank: 13/22
Findings: 2
Award: $99.33
🌟 Selected for report: 0
🚀 Solo Findings: 0
defsec
The _peek function in the contract Cvx3CrvOracle.sol fetches the asset price from a Chainlink aggregator using the latestRoundData function. However, there are no checks on timestamp, resulting in stale prices. The oracle wrapper calls out to a chainlink oracle receiving the latestRoundData(). It then checks freshness by verifying that the answer is indeed for the last known round.
Stale prices could put funds at risk. According to Chainlink's documentation, This function does not error if no answer has been reached but returns 0, causing an incorrect price fed to the PriceOracle. The external Chainlink oracle, which provides index price information to the system, introduces risk inherent to any dependency on third-party data sources. For example, the oracle could fall behind or otherwise fail to be maintained, resulting in outdated data being fed to the index price calculations of the AMM. Oracle reliance has historically resulted in crippled on-chain systems, and complications that lead to these outcomes can arise from things as simple as network congestion.
"https://github.com/code-423n4/2022-01-yield/blob/main/contracts/Cvx3CrvOracle.sol#L120"
Consider to add checks on the return data (Timestamp) with proper revert messages if the price is stale in the timestamp, for example:
(uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = ETH_CHAINLINK.latestRoundData(); require(price > 0, "Chainlink price <= 0"); require(answeredInRound >= roundID, "..."); require(timeStamp != 0, "...");
#0 - devtooligan
2022-02-01T01:13:08Z
dup of #2
#1 - GalloDaSballo
2022-02-18T00:57:24Z
Duplicate of #136
defsec
'immutable' greatly reduces gas costs. There are variables that do not change so they can be marked as immutable to greatly improve the gas costs.
Code Review
Mark variables as immutable.
#0 - devtooligan
2022-02-01T02:26:04Z
dup of #42
defsec
++i is more gas efficient than i++ in loops forwarding.
https://github.com/code-423n4/2022-01-yield/blob/main/contracts/ConvexYieldWrapper.sol#L111
Code Review
It is recommend to use unchecked{++i} and change i declaration to uint256.
#0 - alcueca
2022-02-02T16:11:19Z
Duplicate of #14
17.3157 USDC - $17.32
defsec
!= 0
is a cheaper operation compared to > 0
, when dealing with uint.
https://github.com/code-423n4/2022-01-yield/blob/main/contracts/ConvexStakingWrapper.sol#L165 https://github.com/code-423n4/2022-01-yield/blob/main/contracts/ConvexStakingWrapper.sol#L182 https://github.com/code-423n4/2022-01-yield/blob/main/contracts/ConvexYieldWrapper.sol#L128 https://github.com/code-423n4/2022-01-yield/blob/main/contracts/ConvexYieldWrapper.sol#L142
Code Review
Use "!=0" instead of ">0" for the gas optimization.
#0 - alcueca
2022-02-02T16:18:38Z
Duplicate of #57