Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistency in view functions can lead to users believing they’re due for more BKD rewards #150

Open
code423n4 opened this issue Jun 3, 2022 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

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.

Proof of Concept

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().

Tools Used

Manual review

Recommended Mitigation Steps

To make the logic consistent, claimableRewards() needs if (inflationRecipient == address(0)) added to it.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jun 3, 2022
code423n4 added a commit that referenced this issue Jun 3, 2022
@chase-manning chase-manning added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jun 6, 2022
@GalloDaSballo
Copy link
Collaborator

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

@GalloDaSballo
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants