Users can front-run the process of locking tokens in `CvxConvergenceLocker.sol`

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xc41587ebfa586d7978d4184b491d91f84c784f50fdd46923c257aca71f4d3457 Severity: medium

Description: Description\

The current functionality of CvxConvergence allows users to mint an amount of cvgCVX to the account in exchange of the same amount in CVX. The problem is in isLock parameter that allows the users to not lock the tokens immediately and then possibly front-run lockCvx() call.

Attack Scenario\ Describe how the vulnerability can be exploited.

Currently users may call mint to mint cvgCVX and lock the corresponding amount of CVX:

https://github.com/hats-finance/Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb/blob/main/contracts/Staking/Convex/cvgCVX/CvxConvergenceLocker.sol#L209

https://github.com/hats-finance/Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb/blob/main/contracts/Staking/Convex/cvgCVX/CvxConvergenceLocker.sol

The problem is in the isLock parameter that allows the users not to lock the tokens in exchange of paying the fees. But they are still supposed to be locked anyway after somebody calls lockCVX():

https://github.com/hats-finance/Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb/blob/main/contracts/Staking/Convex/cvgCVX/CvxConvergenceLocker.sol#L112-115

 function lockCvx() public {
        CVX_LOCKER.lock(address(this), cvxToLock, 0);
        cvxToLock = 0;
    }

The problem is that the users may mint the tokens and then front-run the call to lockCvx() after waiting some time because the call will not be immediate. By doing this, they can bypass this requirement of locking all the tokens:

https://github.com/hats-finance/Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb/blob/main/contracts/Staking/Convex/cvgCVX/CvxConvergenceLocker.sol#L223-228

 cvxToLock += amount;
        if (isLock) {
            lockCvx();
        }

        _mint(account, amount);

Recommendation

Consider not allowing the users to be able to avoid locking the tokens and get the rewards with that.

[PlamenTSV]

Whether he chooses to lock them on mint or to lock them later one, the tokens will always get locked. There is no condition that would brick his minting in case of no tokens to locked, so I do not really understand where the issue is? The outcome is the same.

[rodiontr]

Whether he chooses to lock them on mint or to lock them later one, the tokens will always get locked. There is no condition that would brick his minting in case of no tokens to locked, so I do not really understand where the issue is? The outcome is the same.

I don't understand what you're trying to say here "There is no condition that would brick his minting in case of no tokens to locked". The issue here is that the user can avoid making his tokens to be locked as the function is needed to be triggered by some external party and the attacker can supposedly use these tokens to get the rewards at the same time while not locking his tokens. And if lockCvx() is triggered, he can just watch the mempool and then front-run

[PlamenTSV]

You are saying a user can mint some tokens without locking and then front-run the lock and do what? He cannot withdraw his tokens, he would just pay a fee for an action that is going to happen anyway. If you think he can cheat more rewards, he cannot, because pullRewards takes this into account: if (rewardConfiguration.token == CVX) balance -= cvxToLock cvxToLock always increments on mint whether you choose to lock them during minting or let someone else do it. I do not see the issue, give me a PoC or some step-by-step scenario if you believe otherwise.