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

5 stars 0 forks source link

Underlying assets may be modified during execution #328

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#L324 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L344

Vulnerability details

Tokens whose market value is related to some stateful property may be manipulated in certain scenarios during order execution. This requires a number of specific assumptions, and it's not really possible to implement contract-level safeguards against it, but I think it's worth considering the scenario, documenting it for your end users, and watching out for it in the wild.

Scenario:

  1. Eve owns an ERC721 token whose fair market value is related in part to some additional stateful property, like an unclaimed airdrop or the ability to mint additional tokens. (For example, unclaimed $APE token for a Bored Ape). The market value of her token is 10 ETH for the token itself plus 5 ETH related to these external attributes.
  2. Eve signs and submits a put order with a strike price of 15 ETH and premium of 0.1 ETH and uses a token with a before transfer callback hook as the baseAsset. (For example, an ERC777 or nonstandard ERC20). She signs her order from a smart contract that can recieve the before transfer callback.
  3. Bob calls fillOrder to fill Eve's order and collects the premium. Before Eve's tokens are transferred, her contract recieves the transfer callback. In the callback hook, Eve claims the unclaimed airdrop related to her token. Without the associated tokens, the fair market value of the token is now 10 ETH.
  4. Eve calls exercise and exercises her option. She has received 15 ETH from the option, plus 5 ETH from the airdrop claim, minus the 0.1 ETH premium, or 19.9 ETH total. Bob can only recoup 10 ETH of the 15 ETH strike on the open market.

Impact: Malicious makers may trick takers into writing options at above market strike prices for certain stateful tokens.

Recommendation: There is not really a reasonable fix here at the contract level. Instead, I recommend clearly documenting the risks of writing options on tokens with stateful properties for your end users, and keeping an eye out for this behavior in production.

outdoteth commented 2 years ago

Duplicate: It’s possible to flashloan all assets in the contract without paying a protocol fee: https://github.com/code-423n4/2022-06-putty-findings/issues/377

HickupHH3 commented 2 years ago

Scenario described is something I've seen with $APE token and flashloaning from NFTX.