search

code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

[WP-H15] `AmmConvexGauge.sol#poolCheckpoint()` `cvxStakedIntegral` can be manipulated by the attacker #132

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/tokenomics/AmmConvexGauge.sol#L185-L212

Vulnerability details

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/tokenomics/AmmConvexGauge.sol#L185-L212

function poolCheckpoint() public virtual override returns (bool) {
    if (killed) {
        return false;
    }
    uint256 timeElapsed = block.timestamp - uint256(ammLastUpdated);
    uint256 currentRate = IController(controller).inflationManager().getAmmRateForToken(
        ammToken
    );
    uint256 crvEarned = IERC20(crv).balanceOf(address(this)) -
        _preClaimRewardsCrvEarned +
        crvRewardsContract.earned(address(this));
    uint256 cvxEarned = getCvxMintAmount(crvEarned);

    // Update the integral of total token supply for the pool
    if (totalStaked > 0) {
        if (inflationRecipient == address(0)) {
            ammStakedIntegral += (currentRate * timeElapsed).scaledDiv(totalStaked);
        } else {
            perUserShare[inflationRecipient] += currentRate * timeElapsed;
        }
        crvStakedIntegral += (crvEarned - _crvLastEarned).scaledDiv(totalStaked);
        cvxStakedIntegral += (cvxEarned - _cvxLastEarned).scaledDiv(totalStaked);
    }
    _crvLastEarned = crvEarned;
    _cvxLastEarned = cvxEarned;
    ammLastUpdated = uint48(block.timestamp);
    return true;
}

In the current implementation, crvEarned is the new crv reward earned since the last claim.

However, anyone can send crv tokens to the contract to increase crvEarned.

Since cvxEarned is based on crvEarned, by increasing crvEarned, cvxStakedIntegral can be increased artificially.

When the increased cvx is worth more than the crv tokens sent, the attacker can net a profit from it.

PoC

Given:

  1. The attacker deposits a lot of tokens and take 80% of totalStaked;
  2. The attacker sends 10,000 CRV to the contract, the claimRewards():
  1. The attacker receives 1,600 CVX, worth 41,600 USD

Recommendation

Consider updating the cvxStakedIntegral after crvRewardsContract.getReward(); with the actual amount of CVX tokens received.

gzeoneth commented 2 years ago

Normally the gauge should distribute CRV and CVX proportional to user's share of totalStaked. In case there are 2000 CVX in the contract, an attacker who own 80% of the share should receive 1600 CVX without an attack. It looks like its possible for the attacker to donate additional 5000 CRV (assuming 90M CVX total supply) such that he can drain the remaining 400 CVX in the contract, which he can recover 5000 * 80% = 4000 CRV for a total cost of 1000 CRV.

gzeoneth commented 2 years ago

Invalid due to out-of-scope. https://github.com/code-423n4/2022-04-backd/blob/main/README.md