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: 7/22
Findings: 1
Award: $881.09
π Selected for report: 1
π Solo Findings: 0
π Selected for report: GeekyLumberjack
881.0898 USDC - $881.09
GeekyLumberjack
uint256[2] memory depositedBalance;
is defined at the beginning of _checkpointAndClaim only one depositedBalance
slot is being filed and then the entire array gets passed into _calcRewardIntegral()
and _calcCvxIntegral()
along with an array of two _accounts
. Having only one of the depositedBalance
and two _accounts
may cause loss in rewards for the second account. This function is currently only used in GetReward()
which is passing in a zero address as the second address.
#0 - alcueca
2022-02-02T12:23:11Z
Yikes, Abracadabra's code leaves a bit to be desired. We were wrong in trusting them.
#1 - GalloDaSballo
2022-02-18T15:09:54Z
@alcueca and @iamsahu are you sure the finding is valid?
From checking the code, _checkpointAndClaim
Β is called only once, by getReward
This does a checkpoint, where the second account is always 0, as such the check the warden mentions (second account balance) is not necessary.
The second balance is always zero as the second account is always address(0)
Can you please check my counter-argument and let me know if you actually need to have a second balance?
#2 - GalloDaSballo
2022-02-18T15:10:46Z
Images to make conversation easier: <img width="571" alt="Screenshot 2022-02-18 at 16 10 07" src="https://user-images.githubusercontent.com/13383782/154709015-74813941-e4d6-4e79-a715-432b7d6e52c7.png"> (Second account is always 0)
<img width="746" alt="Screenshot 2022-02-18 at 16 10 17" src="https://user-images.githubusercontent.com/13383782/154709050-6844bbce-ea3f-4754-a656-48f4ad80003b.png"> So the 2nd balance being 0 is a optimization#3 - alcueca
2022-02-21T09:25:59Z
It is true that calling _checkpointAndClaim
with two accounts would cause a loss. However, it is also true that the current code doesn't ever call checkpointAndClaim
with two accounts. Considering that, you could downgrade the severity of the bug (_checkpointAndClaim
should at least revert if passed a second account).
#4 - GalloDaSballo
2022-02-21T14:12:54Z
From my understanding, the finding originally had no impact.
While technically correct (it ignores the second account), because it was used exclusively for single accounts, no loss of funds could happen.
Given that the sponsor:
I'll downgrade to Low Severity