sherlock-audit / 2023-10-looksrare-judging

6 stars 6 forks source link

phenom - Using block.timestamp as the deadline/expiry invites MEV #97

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

phenom

medium

Using block.timestamp as the deadline/expiry invites MEV


name: Using block.timestamp as the deadline/expiry invites MEV about: Using block.timestamp as the deadline/expiry invites MEV title: "Using block.timestamp as the deadline/expiry invites MEV" labels: "" assignees: ""

Summary

Using block.timestamp as the deadline/expiry invites MEV

Vulnerability Detail

In the smart contract, various operations involve specifying block.timestamp as the expiry or deadline for a given transaction. While it may be intuitive to assume that using block.timestamp in this context implies 'require immediate execution,' it actually means 'whatever block this transaction appears in, I'm comfortable with that block's timestamp.'

This approach opens the door to potential Miner Extractable Value (MEV) opportunities. Malicious miners can strategically hold a transaction until conditions become most favorable for their own profit. For instance, they might wait for the transaction to incur the maximum allowable slippage, take advantage of slippage parameters, or trigger other actions, such as liquidations, to their advantage.

To avoid these MEV opportunities, it is crucial to refrain from using block.timestamp as the deadline/expiry and instead select timestamps off-chain. Furthermore, allowing the caller to specify timestamps provides greater control and reduces the potential for unnecessary MEV exploitation.

Impact

The impact of using block.timestamp as the deadline/expiry in transactions is significant:

These consequences can result in financial losses and undermine the intended functionality of the smart contract.

Code Snippet

Instances (9):

  1. Line 422: if (block.timestamp > newMintStart) {
  2. Line 428: if (block.timestamp >= currentMintStart) {
  3. Line 436: if (block.timestamp > newMintEnd || newMintEnd < mintEnd) {
  4. Line 469: if (block.timestamp < mintStart || block.timestamp > mintEnd) {
  5. Line 469: if (block.timestamp < mintStart || block.timestamp > mintEnd) {
  6. Line 502: if (block.timestamp < mintEnd) {
  7. Line 561: bool conditionThree = currentRoundId == 0 && block.timestamp > uint256(mintEnd).unsafeAdd(36 hours);
  8. Line 594: if (block.timestamp < uint256(gameInfo.randomnessLastRequestedAt).unsafeAdd(1 days)) {
  9. Line 1308: gameInfo.randomnessLastRequestedAt = uint40(block.timestamp);

Tool used

Manual Review

Recommendation

To mitigate the vulnerability related to MEV exploitation, it is strongly recommended to avoid using block.timestamp as the deadline or expiry for transactions within the smart contract. Instead, timestamps should be selected off-chain, and callers should have the capability to specify timestamps. This approach enhances control, predictability, and security in transaction execution and reduces the potential for MEV-related issues.

nevillehuang commented 1 year ago

Invalid, misunderstanding of how block.timestamp works