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: 9/22
Findings: 2
Award: $195.46
🌟 Selected for report: 1
🚀 Solo Findings: 0
TomFrenchBlockchain
Usage of stale prices when querying chainlink oracles.
Cvx3CrvOracle queries chainlink oracles for the prices of DAI, USDC and USDT, however it doesn't require that the response is fresh by checking which round the answer was provided in.
See previous similar findings: https://github.com/code-423n4/2021-12-perennial-findings/issues/24 https://github.com/code-423n4/2021-10-mochi-findings/issues/87 https://github.com/code-423n4/2021-04-maple-findings/issues/82
Also see this protection added elsewhere in yield protocol codebase as a result of a previous Code4rena finding: https://github.com/yieldprotocol/vault-v2/pull/187
Set as medium severity based on previous precedent. (Improper valuation of collateral for liquidation purposes could result in preventing liquidation of underwater vaults, however this requires an unlikely scenario of chainlink being stale). Could also be argued as low based on maple finding.
Implement a similar protection on this function as done in vault-v2 codebase.
#0 - iamsahu
2022-02-01T19:30:03Z
Duplicate of #136
TomFrenchBlockchain
Gas savings
We have a number of variables in ConvexStakingWrapper which appear to only be set on construction and never again.
We can then make use of the immutable
keyword to save on gas.
Make use of immutable keyword for variables which are only set on construction
#0 - iamsahu
2022-01-30T06:17:50Z
Immutable variables cannot be read during contract creation time. Making the suggested variables immutable would lead to https://github.com/code-423n4/2022-01-yield/blob/e946f40239b33812e54fafc700eb2298df1a2579/contracts/ConvexStakingWrapper.sol#L77-L78 to fail.
#1 - devtooligan
2022-02-02T16:22:32Z
dup of #124
🌟 Selected for report: Dravee
Also found by: TomFrenchBlockchain, robee
25.6528 USDC - $25.65
TomFrenchBlockchain
Detailed description of the impact of this finding.
In Cvx3CrvOracle._peek
we SLOAD ethId
and cvx3CrvId
2-3 times depending on whether we're querying ETH/cvx3Crv or vice versa.
We can then just cache these into memory once at the beginning of the function to avoid some of these SLOADs.
Load ethId
and cvx3CrvId
into variables in memory to avoid repeated SLOADs.
#0 - iamsahu
2022-02-01T14:24:59Z
#70
🌟 Selected for report: TomFrenchBlockchain
95.0104 USDC - $95.01
TomFrenchBlockchain
gas costs
L116 of Cvx3CrvOracle enforces for the rest of the function call that base == ethId <-> quote == cvx3CrvId
However on L137 we check both these conditions again.
We could check just one of these and then rely on the require condition on 116 to enforce the other one. This will prevent us having to SLOAD ethID
again
Change L137 to if (base == cvx3CrvId) {
#0 - GalloDaSballo
2022-02-13T01:56:13Z
Agree with the finding, because ethId
is a hot storage variable, the finding will save 100 gas plus the cost of the extra checks (ballpark around 20 /30 gas)