code-423n4 / 2022-06-putty-findings

5 stars 0 forks source link

Option Cannot Be Exercised Due To Unbounded Array Within An Order #294

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L268 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L389

Vulnerability details

Note: This issue only affects the "Short Put" workflow

Vulnerability Details

An order could contain an unbounded number of floorTokens, erc20Assets and/or erc721Assets.

Although multiple unbounded array issues exist within the in-scope contracts, this issue is different in a way that it demostrated that it will result in loss of funds for the users (long option holders) as they will not be able to exercise their option as shown in the Proof-of-Concept section.

This issue only affects the "Short Put" workflow, where an order maker creates a short put order, and an order taker attempts to fill the short put order. This is because during the "Short Put" workflow:

Proof-of-Concept

The following illustrate an example of how an attacker could make use of the unbounded erc20Assets to cause the victim to loss funds.

Note: Fee is ignored for simplicity.

  1. Alice (Attacker) create a Short Put order with the following configuration:

Strike Price = 100 ETH

Premium = 5 ETH

isLong = False

isCall = False

baseAsset = WETH

ERC721Asset[] = an array of 1000 NFTs (or any large number of NFTs that cause out-of-gas error)

  1. Bob (Victim) decided to fill Alice's order by calling PuttyV2.fillOrder. Thus, Bob send 5 ETH (premium) to Alice, and Alice transfer 100 ETH (strike price) to PuttyV2 contract for escrow.

  2. At this point, Bob holds a Long Put order, while Alice holds a Short Put order.

  3. At certain point of time, Bob decided to exercise his Long Put option by calling PuttyV2.exercise function. In layman term, Bob wanted to exercise his option to sell NFTs at the strike price at this point of time.

  4. Thus, the PuttyV2 contract will attempt to transfer 100 ETH (strike price) escrowed within the contract to Bob, and the PuttyV2 contract will "pull" 1000 NFTs from Bob.

  5. However, due to large amount of NFTs that need to be transferred from Bob to PuttyV2 contract. Thus, the PuttyV2.exercise function will always revert due to out-of-gas error. As a result, there is no way for Bob to exercise his Long Put option.

  6. When the Bob's Long Put option expired, Alice proceeds to reclaim back the 100 ETH that she previously escrowed within the PuttyV2 contract.

  7. Alice earns the 5 ETH premium

Alice could initiate multiple malicious orders at a time to 'fish' for victims in Putty Protocol. As long as there is someone willing to fill her order, she will earn the premium in a risk-free environment because the order takers can never exercise their option.

Alice could make the orders more attractive by configuring the order in a manner that will attract potential takers.

Impact

The order taker (victim) would lost their funds as they paid for an option that cannot be exercised.

Recommended Mitigation Steps

It is recommended to restrict the number of floorTokens, erc20Assets and erc721Assets within an order to a upper limit (e.g. 10).

Although client-side or off-chain might have already verified that the number of floorTokens, erc20Assets and erc721Assets do not exceed a certain limit within an order, simply relying on client-side and off-chain validations are not sufficient. It is possible for an attacker to bypass the client-side and off-chain validations and interact directly with the contract. Thus, such validation must also be implemented on the on-chain contracts.

outdoteth commented 2 years ago

Duplicate: Unbounded loop can prevent put option from being exercised: https://github.com/code-423n4/2022-06-putty-findings/issues/227