sherlock-audit / 2024-05-sophon-judging

7 stars 6 forks source link

dimi6oni - Lack of flash loan protection leads to reward manipulation #207

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

dimi6oni

high

Lack of flash loan protection leads to reward manipulation

Summary

InSophonFarming contract, users earn points based on their staking activity. These points are later used to determine the distribution of rewards, typically in the form of tokens. The key functions involved in this process include deposit, withdraw, increaseBoost, and the internal calculations for accumulating points and determining pending rewards. The contract does not include checks or mechanisms to mitigate the risk of flash loan attacks, where an attacker could borrow a large amount of tokens, manipulate pool states, and quickly repay the loan within the same transaction.

Vulnerability Detail

  1. Flash Loan Execution: An attacker takes out a flash loan to borrow a large amount of tokens.
  2. Temporary Balance Inflation: The attacker stakes the borrowed tokens by calling the deposit function. This action temporarily inflates their balance and consequently their share of the pool.
  3. Points Accumulation: Because the attacker now has a significantly larger balance, they accumulate points at an accelerated rate. This is due to the function _deposit updating their amount and recalculating accPointsPerShare.
  4. Immediate Reward Claim: The attacker may call withdraw or increaseBoost to realize the points and potentially boost their balance further, depending on the contract's exact reward realization mechanism.
  5. Loan Repayment: Within the same transaction, the attacker withdraws the staked tokens and repays the flash loan.
  6. Excessive Points Accumulation: Despite the attacker's balance being temporarily inflated, they keep the points earned during the attack. This results in an unfair distribution of points, giving the attacker more points than they would have legitimately earned.

    Impact

  7. Unfair Reward Distribution: The attacker ends up with more points than they deserve, which later translates into a larger share of the rewards. This reduces the reward pool available to legitimate stakers.
  8. Economic Disincentive: Honest users receive fewer rewards than expected, leading to dissatisfaction and potentially driving them away from the protocol.
  9. Protocol Integrity: Repeated exploitation of this vulnerability can drain the reward pool, undermining the protocol's integrity and sustainability.

    Code Snippet

    SophonFarming::deposit

https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/farm/SophonFarming.sol#L443-L451

SophonFarming::increaseBoost

https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/farm/SophonFarming.sol#L631-L681

SophonFarming::withdraw

https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/farm/SophonFarming.sol#L699-L742

Tool used

Manual Review

Recommendation

Implement mechanisms to track and limit the impact of flash loans, such as time-weighted average balances (TWAB), flash loan guards or minimum staking periods, to ensure that rewards are distributed fairly based on long-term staking.

Time-Weighted Average Balances (TWAB) Implementation:

TWAB Data Structures

struct TWAB {
    uint256 balance;
    uint256 timestamp;
}

struct UserInfo {
    uint256 amount; // Current amount of LP tokens the user has
    uint256 boostAmount; // Boosted value purchased by the user
    uint256 depositAmount; // Remaining deposits not applied to boost purchases
    uint256 rewardSettled; // Reward settled
    uint256 rewardDebt; // Reward debt
    TWAB[] twabs; // Array of TWABs
}

Helper Functions

The _updateTWAB function updates the user's time-weighted average balance whenever they interact with the contract by depositing, withdrawing, or increasing their boost. This ensures that the balance history is accurately recorded. The _getTWAB function calculates the average balance over a specified period. This can be used to determine the reward distribution based on the average balance rather than the instantaneous balance, mitigating the risk of flash loan attacks.

function _updateTWAB(UserInfo storage user) internal {
    uint256 currentTime = block.timestamp;
    if (user.twabs.length == 0 || user.twabs[user.twabs.length - 1].timestamp < currentTime) {
        user.twabs.push(TWAB({ balance: user.amount, timestamp: currentTime }));
    } else {
        TWAB storage lastTwab = user.twabs[user.twabs.length - 1];
        lastTwab.balance = user.amount;
    }
}

function _getTWAB(UserInfo storage user, uint256 startTime, uint256 endTime) internal view returns (uint256) {
    uint256 totalWeightedBalance = 0;
    uint256 totalDuration = 0;

    for (uint256 i = 0; i < user.twabs.length; i++) {
        TWAB storage twab = user.twabs[i];
        if (twab.timestamp >= endTime) break;
        uint256 nextTimestamp = (i + 1 < user.twabs.length) ? user.twabs[i + 1].timestamp : endTime;
        uint256 duration = nextTimestamp - twab.timestamp;
        totalWeightedBalance += twab.balance * duration;
        totalDuration += duration;
    }

    return totalDuration > 0 ? totalWeightedBalance / totalDuration : 0;
}

Modified deposit, withdraw, and increaseBoost Functions

The deposit, withdraw, and increaseBoost functions are modified to call _updateTWAB after every balance-changing operation to ensure the TWAB is always up to date.

function deposit(uint256 _pid, uint256 _amount, uint256 _boostAmount) external nonReentrant {
    poolInfo[_pid].lpToken.safeTransferFrom(
        msg.sender,
        address(this),
        _amount
    );

+   UserInfo storage user = userInfo[_pid][msg.sender];
+   _updateTWAB(user);

    _deposit(_pid, _amount, _boostAmount);
}
function _deposit(uint256 _pid, uint256 _depositAmount, uint256 _boostAmount) internal {
    // Existing deposit logic including points calculation

    // Update TWAB after deposit changes
+   _updateTWAB(user);

    emit Deposit(msg.sender, _pid, _depositAmount, _boostAmount);
}
function withdraw(uint256 _pid, uint256 _withdrawAmount) external nonReentrant {
    if (isWithdrawPeriodEnded()) {
        revert WithdrawNotAllowed();
    }
    if (_withdrawAmount == 0) {
        revert WithdrawIsZero();
    }

    PoolInfo storage pool = poolInfo[_pid];
    UserInfo storage user = userInfo[_pid][msg.sender];
    updatePool(_pid);

    uint256 userDepositAmount = user.depositAmount;

    if (_withdrawAmount == type(uint256).max) {
        _withdrawAmount = userDepositAmount;
    } else if (_withdrawAmount > userDepositAmount) {
        revert WithdrawTooHigh(userDepositAmount);
    }

    uint256 userAmount = user.amount;
    user.rewardSettled =
        userAmount * pool.accPointsPerShare / 1e18 +
        user.rewardSettled -
        user.rewardDebt;

    user.depositAmount = userDepositAmount - _withdrawAmount;
    pool.depositAmount = pool.depositAmount - _withdrawAmount;

    userAmount = userAmount - _withdrawAmount;

    user.amount = userAmount;
    pool.amount = pool.amount - _withdrawAmount;

    pool.lpToken.safeTransfer(msg.sender, _withdrawAmount);

    user.rewardDebt = userAmount * pool.accPointsPerShare / 1e18;
+   _updateTWAB(user);

    emit Withdraw(msg.sender, _pid, _withdrawAmount);
}
function increaseBoost(uint256 _pid, uint256 _boostAmount) external nonReentrant {
    if (isFarmingEnded()) {
        revert FarmingIsEnded();
    }

    if (_boostAmount == 0) {
        revert BoostIsZero();
    }

    uint256 maxAdditionalBoost = getMaxAdditionalBoost(msg.sender, _pid);
    if (_boostAmount > maxAdditionalBoost) {
        revert BoostTooHigh(maxAdditionalBoost);
    }

    PoolInfo storage pool = poolInfo[_pid];
    UserInfo storage user = userInfo[_pid][msg.sender];
    updatePool(_pid);

    uint256 userAmount = user.amount;
    user.rewardSettled =
        userAmount * pool.accPointsPerShare / 1e18 +
        user.rewardSettled -
        user.rewardDebt;

    heldProceeds[_pid] = heldProceeds[_pid] + _boostAmount;

    user.depositAmount = user.depositAmount - _boostAmount;
    pool.depositAmount = pool.depositAmount - _boostAmount;

    uint256 finalBoostAmount = _boostAmount * boosterMultiplier / 1e18;

    user.boostAmount = user.boostAmount + finalBoostAmount;
    pool.boostAmount = pool.boostAmount + finalBoostAmount;

    userAmount = userAmount + finalBoostAmount - _boostAmount;

    user.amount = userAmount;
    pool.amount = pool.amount + finalBoostAmount - _boostAmount;

    user.rewardDebt = userAmount * pool.accPointsPerShare / 1e18;
+   _updateTWAB(user);

    emit IncreaseBoost(msg.sender, _pid, finalBoostAmount);
}
sherlock-admin3 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

invalid because flash loan attack won't work on time based point farming