Platform: Code4rena
Start Date: 31/03/2022
Pot Size: $75,000 USDC
Total HM: 7
Participants: 42
Period: 7 days
Judge: Jack the Pug
Total Solo HM: 5
Id: 102
League: ETH
Rank: 27/42
Findings: 1
Award: $130.37
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: rayn
Also found by: 0xDjango, 0xkatana, 0xkowloon, BouSalman, CertoraInc, Dravee, Funen, Hawkeye, IllIllI, Jujic, Kenshin, Kthere, Meta0xNull, Sleepy, TerrierLover, async, aysha, berndartmueller, catchup, cccz, cmichel, csanuragjain, danb, defsec, georgypetrov, hake, hubble, kenta, kyliek, pauliax, rfa, robee, sahar, shenwilly, teryanarmen
130.3737 USDC - $130.37
OraclePassThrough
No zero address check for scalingPriceOracle neither in updateScalingPriceOracle nor in constructor. --------- USES A POINTER, MIGHT BE VOID.
ScalingPriceOracle.constructor
Accidentally setting some of the varialbes in the constructor to zero like _currentMonth or _oracle might require relaunching the contract.
RateLimited.setRateLimitPerSecond
_rateLimitPerSecond and _maxRateLimitPerSecond have no zero checks. The lack of such safe guards gives the opportunity for mistakenly setting one or both to zero.
pcvDeposit.withdraw(to, amountOut);
No zero address check for to might lead to the undesired burning of underlying tokens on withdraw.
DOMAIN_SEPARATOR is not recalculated in case of a hard fork.Volt.DOMAIN_SEPARATOR
In case of a hard fork there would be a change to chainId and DOMAIN_SEPARATOR would become invalid in one of the forked chains.
I recommend using the implementation from OpenZeppelin, which recalculates the domain separator if the current chain ID doesnt match the cached chain ID. An alternative could be implementing a fix like this.
chainId := chainid() is outdated.The use of assembly to get chainId is unnecessary and can be done through block.chainid in pure solidity. I believe this would improve readability.
References: solidity-docs,implementation-example
#0 - ElliotFriedman
2022-04-05T22:24:32Z
Low #1 is not an issue as the system assumes that the deployer will not specify invalid params, and if they do, the contract can simply be redeployed. The system also does the least amount of checks possible to save gas and the UI will validate all params before they are sent.
Low #2 Good catch. I'm unsure if this will be patched due to the added gas overhead of using an upgradeable mechanism.
Low #3 Good catch.