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

mVeNFT DOS - can't trigger the vote function #21

Open
c4-bot-2 opened this issue Sep 25, 2024 · 4 comments
Open

mVeNFT DOS - can't trigger the vote function #21

c4-bot-2 opened this issue Sep 25, 2024 · 4 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 downgraded by judge Judge downgraded the risk level of this issue M-01 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_09_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

Comments

@c4-bot-2
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-09-fenix-finance/blob/main/contracts/core/VoterUpgradeableV2.sol#L485
https://github.com/code-423n4/2024-09-fenix-finance/blob/main/contracts/core/VoterUpgradeableV2.sol#L448

Vulnerability details

Description

The VoterUpgradeableV2.sol contract has the function attachToManagedNFT(), users use it to delegate their veFNX voting power to a mVeNFT.
One of the things this function does after receiving the new voting power is sub-call to _poke() and it will update the Last voted Timestamp of the mVeNFT

lastVotedTimestamps[tokenId_] = _epochTimestamp() + 1;

At this point, the mVeNFT can't trigger the vote function until the next epoch starts due to the _checkVoteDelay().
even this check inside the vote() doesn't help in this case

if (!managedNFTManagerCache.isWhitelistedNFT(tokenId_)) {
            _checkEndVoteWindow();
        }

However, to make things worse this protocol is deployed on Blast transactions are too cheap
malicious users can keep creating new locks every epoch with one wei in amount to bypass the zero check

File: VotingEscrowUpgradeableV2.sol#_createLock()

 LibVotingEscrowValidation.checkNoValueZero(amount_);

then at the start of every new epoch (after the start of the voting window) just call attachToManagedNFT()
By doing this it keeps forcing the mVeNFT to vote to the same gauges.

Impact

DOS attack.
mVeNFT can't invoke the vote function to change the weight of gauges.
mVeNFT can't reset its votes.

Tools Used

Manual Review

Recommended Mitigation Steps

One solution is to not check the vote delay, However, I believe this comes with some trade-off

    function vote(
        uint256 tokenId_,
        address[] calldata poolsVotes_,
        uint256[] calldata weights_
    ) external nonReentrant onlyNftApprovedOrOwner(tokenId_) {
        if (poolsVotes_.length != weights_.length) {
            revert ArrayLengthMismatch();
        }
        bool x = managedNFTManagerCache.isWhitelistedNFT(tokenId_);
        if (!x) {
        _checkVoteDelay(tokenId_);
        }

        _checkStartVoteWindow();
        IManagedNFTManager managedNFTManagerCache = IManagedNFTManager(managedNFTManager);
        if (managedNFTManagerCache.isDisabledNFT(tokenId_)) {
            revert DisabledManagedNft();
        }
        if (!x) {
            _checkEndVoteWindow();
        }
        _vote(tokenId_, poolsVotes_, weights_);
        _updateLastVotedTimestamp(tokenId_);
    }

Assessed type

DoS

@c4-bot-2 c4-bot-2 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Sep 25, 2024
c4-bot-5 added a commit that referenced this issue Sep 25, 2024
@c4-bot-13 c4-bot-13 added 🤖_09_group AI based duplicate group recommendation 🤖_primary AI based primary recommendation labels Sep 25, 2024
@c4-judge
Copy link

alcueca marked the issue as duplicate of #9

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

alcueca marked the issue as satisfactory

@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 downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Sep 30, 2024
@c4-judge
Copy link

alcueca changed the severity to 2 (Med Risk)

@c4-judge
Copy link

alcueca marked the issue as selected for report

@c4-judge c4-judge reopened this Sep 30, 2024
@c4-judge c4-judge added primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report labels Sep 30, 2024
@C4-Staff C4-Staff added the M-01 label Oct 4, 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 downgraded by judge Judge downgraded the risk level of this issue M-01 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_09_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
Projects
None yet
Development

No branches or pull requests

4 participants