search

code-423n4 / 2022-05-opensea-seaport-findings

1 stars 0 forks source link

Fulfill transactions that are not protected with a deadline may lead to unfavorable trade. #139

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Consideration.sol#L76 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Consideration.sol#L108

Vulnerability details

Impact

A fulfill transaction of order with descending/ascending amount should be protected by the deadline.

The price of an order with a descnding amount is sensitive to the time. Letting users make such a trade without providing the deadline would lead to unfavorable results. As opensea is facing different groups of users, developers should take one step further to protect users.

  1. Will users set up an "auction" and make the order really sensitive to time?
  2. Will users try to send such transactions during network congestion?
  3. Will users complain about making bad trades and blame the protocol?

We consider the answer is yes to the above questions.

A possible scenario:

  1. Users found a cool NFT project on opensea. The project claims that the JPEG will be 10x price in an hour. APE IN!!
  2. This project is so cool that too many people wanna buy and cause the network congestion and even DOS the etherscan.
  3. The etherscan finally came back an hour ago. The user found that he spent 10x the price than he should.
  4. Make a long tweet. “NFT is a scam. Here's why. (1/42)”

Proof of Concept

As stated above.

Tools Used

Manual inspection

Recommended Mitigation Steps

Recommend the team to add an optional deadline parameters in fulfillOrder, fulfillBasicOrder fulfillAdvancedOrder.

0age commented 2 years ago

This is already supported by using matchOrders and setting the endTime on the fulfiller's order.

0xleastwood commented 2 years ago

As the sponsor has noted, matchOrders allows the fulfillers to be protected with a deadline by setting endTime in their order.

0xleastwood commented 2 years ago

To expand on this, Verifiers._verifyTime() will check that block.timestamp is within the bounds of startTime and endTime. If the pending tx is left unconfirmed for some amount of time, the user's order will expire, hence, this is a non-issue.