Yield-Convex contest - GeekyLumberjack's results

Fixed-rate borrowing and lending on Ethereum

General Information

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

Yield

Findings Distribution

Researcher Performance

Rank: 7/22

Findings: 1

Award: $881.09

🌟 Selected for report: 1

πŸš€ Solo Findings: 0

Findings Information

🌟 Selected for report: GeekyLumberjack

Labels

bug
1 (Low Risk)
resolved
sponsor confirmed

Awards

881.0898 USDC - $881.09

External Links

Handle

GeekyLumberjack

Vulnerability details

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:

  • Confirmed
  • Wants to reward the warden
  • Has updated the code by following the warden advice

I'll downgrade to Low Severity

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter