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

Contract does not earn any boosted position rewards in Maverick Connector #1561

Open
c4-bot-3 opened this issue May 17, 2024 · 11 comments
Open
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 grade-b M-01 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_1235_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality

Comments

@c4-bot-3
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-04-noya/blob/9c79b332eff82011dcfa1e8fd51bad805159d758/contracts/connectors/MaverickConnector.sol#L137

Vulnerability details

Summary

The contract has a function in MaverickConnector.sol to call claimBoostedPositionRewards() in order to earn rewards.

Issue:
There will be no boosted position rewards since LP tokens received from adding liquidity in maverick pools are never staked in the boosted position contract.

See here - https://vscode.blockscan.com/ethereum/0x4F24D73773fCcE560f4fD641125c23A2B93Fcb05 and https://docs.mav.xyz/guides/incentives/understanding-boosted-positions. I don't think we even add liquidity to the boosted pool but check.

We first mint LP tokens through our MAV AMM position through this mint() function https://vscode.blockscan.com/ethereum/0x4F24D73773fCcE560f4fD641125c23A2B93Fcb05 and then we stake() it usin this function https://vscode.blockscan.com/ethereum/0x4F24D73773fCcE560f4fD641125c23A2B93Fcb05.

Tools Used

Manual Review

Recommended Mitigation Steps

Assessed type

Error

@c4-bot-3 c4-bot-3 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels May 17, 2024
@c4-bot-13 c4-bot-13 added 🤖_1235_group AI based duplicate group recommendation 🤖_primary AI based primary recommendation labels May 17, 2024
@c4-pre-sort
Copy link

DadeKuma marked the issue as primary issue

@c4-pre-sort
Copy link

DadeKuma marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label May 20, 2024
@HadiEsna
Copy link

I think the severity is med/low since the assets are not at risk, the only impact is that we won't be able to use the boosted positions which we are planning to fix.

@HadiEsna HadiEsna added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label May 22, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Jun 1, 2024

gzeon-c4 changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jun 1, 2024
@gzeon-c4
Copy link

gzeon-c4 commented Jun 1, 2024

Low risk, asset not at risk and MaverickConnector being able to earn staking reward is not a defined behavior.

@mcgrathcoutinho
Copy link

mcgrathcoutinho commented Jun 3, 2024

Hi @gzeon-c4, I believe this issue should of valid Medium-severity.

MaverickConnector being able to earn staking reward is not a defined behavior.

It is a defined behaviour since the MaverickConnector implements a function to claim boosted position rewards. But due to it's incorrect implementation it is not able to do so. If it wasn't the expected behaviour, I do not think the sponsor would have confirmed the issue.

I think this should be a valid Medium-severity issue considering how other issues describing lack of reward/fees claiming functionality or loss of yield/incentives have been judged, which ends up with protocol not receiving their deserved rewards. At the end, the main reason connectors are used is to increase the TVL of the Noya protocol, which the Maverick Connector fails to do (i.e. expected functionality of the contract is impacted and unavailable).

Please consider re-evaluating this issue.

Thank you for your time!

jacobheun pushed a commit that referenced this issue Jun 4, 2024
@gzeon-c4
Copy link

gzeon-c4 commented Jun 7, 2024

will reconsider with a potential re-classification of #1100 and its duplicates later

@mcgrathcoutinho
Copy link

mcgrathcoutinho commented Jun 7, 2024

@gzeoneth Just want to clarify, I do not think this is a dup of #1100 since here the functionality itself is affected i.e. boosted position rewards "cannot be collected" through function claimBoostedPositionRewards() in the first place due to the incorrect implementation while in #1100 it is just saying that the fees/yield which "are collected" are not realtime when reporting TVL.

Similar issues like #976 and #712 have been judged as medium.

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value and removed downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Jun 25, 2024
@c4-judge
Copy link
Contributor

This previously downgraded issue has been upgraded by gzeon-c4

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jun 25, 2024
@c4-judge
Copy link
Contributor

gzeon-c4 marked the issue as satisfactory

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Jun 25, 2024
@c4-judge
Copy link
Contributor

gzeon-c4 marked the issue as selected for report

@C4-Staff C4-Staff added M-01 and removed Q-01 labels Jun 28, 2024
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 grade-b M-01 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_1235_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

8 participants