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

5 stars 0 forks source link

Missing floor token transfer for short call #244

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#L367-L370

Vulnerability details

Missing floor token transfer for short call

Proof of Concept

The floor token should also be transferred in for the short call.

368-369:
            _transferERC20sIn(order.erc20Assets, order.maker);
            _transferERC721sIn(order.erc721Assets, order.maker);

Just like the long call part.

375-377:
            _transferERC20sIn(order.erc20Assets, msg.sender);
            _transferERC721sIn(order.erc721Assets, msg.sender);
            _transferFloorsIn(order.floorTokens, floorAssetTokenIds, msg.sender);

Impact

Lock buyer's NFT asset or funds, and go against the purpose of option.

Tools Used

Mannual analysis.

Recommended Mitigation Steps

Adding the floor token transfer for the short call.

outdoteth commented 2 years ago

Floor tokens should not be transferred in for short calls.

As defined in the spec:

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/spec/fillOrder.md