thisvishalsingh - thisvishalsingh - `NapierPool.sol:: swapPtForUnderlyin,swapUnderlyingForPt, swapUnderlyingForExactBaseLpToken`, and `swapExactBaseLpTokenForUnderlying` functions are suspectible to sandwich attacks due to lack of deadline protection

[sherlock-admin2]

thisvishalsingh

medium

thisvishalsingh - NapierPool.sol:: swapPtForUnderlyin,swapUnderlyingForPt, swapUnderlyingForExactBaseLpToken, and swapExactBaseLpTokenForUnderlying functions are suspectible to sandwich attacks due to lack of deadline protection

name: thisvishalsingh

NapierPool.sol:: swapPtForUnderlyin,swapUnderlyingForPt, swapUnderlyingForExactBaseLpToken, and swapExactBaseLpTokenForUnderlying functions are suspectible to sandwich attacks due to lack of deadline protection"

label: "High Risk"

Summary

The NapierPool.sol contract, which is a pool that allows users to trade between a BasePool LP token and an underlying asset, is currently vulnerable to sandwich attacks due to the absence of a deadline parameter in its swap functions. This vulnerability arises because transactions can be delayed in the mempool, allowing attackers to front-run user transactions and exploit price changes to their advantage.

Vulnerability Detail

In NapierPool.sol contract, there is no explicit deadline parameter protection implemented in the swap functions(swapPtForUnderlyin,swapUnderlyingForPt, swapUnderlyingForExactBaseLpToken, and swapExactBaseLpTokenForUnderlying ) to prevent transactions from being delayed indefinitely and to ensure that they are executed within a specified time frame , also price can be impacted and Front-Running is possible . The contract uses the nonReentrant modifier from the ReentrancyGuard contract to prevent reentrancy attacks, which is a good practice. However, there is no check for a deadline or expiration timestamp in the swap functions or any other functions that could be used to enforce a deadline. The notExpired modifier is used to ensure that the pool has not reached its maturity date:

modifier notExpired() {
    if (maturity <= block.timestamp) revert Errors.PoolExpired();
    _;
}

However, this modifier only checks if the pool's maturity date has passed, and it does not enforce a deadline for individual transactions.

POC

Consider a scenario where a user wants to swap a token for another at a specific price. Without a deadline, the user's transaction could be delayed in the mempool, and by the time it is executed, the price may have changed significantly.

function swapPtForUnderlying(
    uint256 index,
    uint256 ptIn,
    address recipient,
    bytes calldata data
)
    external
    override
    nonReentrant
    notExpired
    returns (uint256 underlyingOut)
{
    // ... existing code ...
}

Detailed Example - how a sandwich attack could be executed against the swapPtForUnderlying function:

1. The victim submits a transaction to swap a principal token (PT) for the underlying asset.
2. The attacker observes the victim's transaction in the memepool.
3. The attacker places a transaction to buy the principal token at a higher price before the victim's transaction is executed.
4. The victim's transaction is executed, buying the principal token at the higher price set by the attacker.
5. The attacker places another transaction to sell the principal token at a higher price after the victim's transaction is executed.
6. The attacker profits from the price difference between steps 3 and 5.

Impact

The lack of deadline protection can lead to significant price impact for users, as their transactions may be executed at a time when the price has significantly changed, resulting in worse outcomes. Additionally, it can make the protocol more susceptible to market manipulation and front-running, which can negatively affect the integrity of the trading mechanism and the overall user experience.

Code Snippet

https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/NapierPool.sol#L324 https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/NapierPool.sol#L395 https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/NapierPool.sol#L455 https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/NapierPool.sol#L489

Tool used

Manual Review

Recommendation

• for example add a deadline parameter to the swapPtForUnderlying function and to mitigate the risk of sandwich attacks:

  function swapPtForUnderlying(
  uint256 index,
  uint256 ptIn,
  address recipient,
  bytes calldata data,
  uint256 deadline
  )
  external
  override
  nonReentrant
  notExpired
  returns (uint256 underlyingOut)
  {
  require(block.timestamp <= deadline, "Deadline has passed");
  // ... existing code ...
  }

  Similarly reconsider other swap functions.

• Use Flashbots to reduce sandwiching probabilities.

[sherlock-admin]

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

takarez commented:

valid: medium(5)

[this-vishalsingh]

@sherlock-admin Without any reason how you can close it? proof me wrong