Platform: Code4rena
Start Date: 27/05/2022
Pot Size: $75,000 USDC
Total HM: 20
Participants: 58
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 15
Id: 131
League: ETH
Rank: 8/58
Findings: 3
Award: $3,472.66
🌟 Selected for report: 1
🚀 Solo Findings: 1
🌟 Selected for report: SmartSek
2732.5332 USDC - $2,732.53
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmConvexGauge.sol#L107-L111 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmConvexGauge.sol#L129-L134
The view functions used for a user to check their claimable rewards vary in their implementation. This can cause users to believe they are due X amount but will receive Y.
If the inflationRecipient is set, then poolStakedIntegral will be incremented in claimableRewards() but not in any other function like allClaimableRewards() or poolCheckpoint().
If a user calls claimableRewards() after the inflationRepient has been set, claimableRewards() will return a larger value than allClaimableRewards() or the amount actually returned by claimRewards().
Manual review
To make the logic consistent, claimableRewards() needs if (inflationRecipient == address(0)) added to it.
#0 - GalloDaSballo
2022-06-21T01:43:54Z
The warden has identified an incorrect implementation in how rewards are shown, while no assets are at risk, the issue identified is a programming mistake that could cause further issues.
For this reason I agree with Medium Severity
#1 - GalloDaSballo
2022-06-22T22:07:38Z
I'm still fairly conflicted on severity as the impact is going to be quite small, however at this time, because the "code is wrong", I still think Medium Severity to be valid
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0xNazgul, 0xf15ers, BowTiedWardens, Chom, Funen, Kaiziron, Kumpa, MiloTruck, Picodes, Ruhum, SecureZeroX, Sm4rty, SmartSek, StyxRave, WatchPug, Waze, asutorufos, bardamu, berndartmueller, c3phas, catchup, cccz, codexploder, cryptphi, defsec, delfin454000, dipp, fatherOfBlocks, gzeon, hake, hansfriese, hyh, masterchief, oyc_109, sach1r0, sashik_eth, shenwilly, simon135, unforgiven
666.4988 USDC - $666.50
Events are emitted at the beginning vs end of the function execution. Temporary variables are/aren't used in the event emission call.
The "safe" function is not the safe version. The unsafe version is the one that reverts in unsuccessful.
SafeApprove() has been deprecated according to OpenZeppelin.
It is recommended to make ownership transfer a two-step process where you firstly set a pending owner address and then accept the pending owner in a second call.
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L227-L232 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/StakerVault.sol#L322 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/StakerVault.sol#L381 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L146 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L131
Sometimes modifier is used. Sometimes require statement is used.
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/KeeperGauge.sol#L57 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L50
#0 - GalloDaSballo
2022-06-20T00:40:20Z
Agree with the catch, consistency is key
Nice catch
Valid but code is safe
## [L-04] Improper ownership transfer Disagree but valid
Non-critical
Valid per industry standard
Agree, nice catch
Agree
This is the type of reports I'd like to see more of, actually specific catches that are custom tailored for the code in scope.
I'll do my best to give extra points to this report which is short, sweet and written by a human being
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, Chom, Dravee, Fitraldys, Funen, Kaiziron, MiloTruck, Picodes, Randyyy, RoiEvenHaim, SecureZeroX, Sm4rty, SmartSek, StyxRave, Tadashi, Tomio, Waze, asutorufos, berndartmueller, c3phas, catchup, csanuragjain, defsec, delfin454000, djxploit, fatherOfBlocks, gzeon, hake, hansfriese, oyc_109, robee, sach1r0, sashik_eth, scaraven, simon135
73.6341 USDC - $73.63
amount is a UINT and therefore can not be less than 0. Change <= to ==
#0 - GalloDaSballo
2022-06-17T22:42:55Z
Should save 3 gas