search

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

5 stars 0 forks source link

missing check of the non-existence of both order sides positions in fillOrder #387

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#L278

Vulnerability details

Impact

Without the checking of existence of orders, one may call the fillOrders multiple times, thus has beseAsset transfered into Putty multiple times but there are no mechanism to exercise the same order multiple times

Proof of Concept

calling fillOrder multiple times: p.fillOrder(order, signature, floorAssetTokenIds); p.fillOrder(order, signature, floorAssetTokenIds);

Tools Used

manual

Recommended Mitigation Steps

add: require(ownerOf(uint256(orderHash)) == address(0), "Order exists already"); require(ownerOf(uint256(oppositeOrderHash)) == address(0), "Opposite Order exists already");

berndartmueller commented 2 years ago

An order can not be filled more than once, as the hash of the order data used as the NFT id can not be re-used. See L303

outdoteth commented 2 years ago

echo @berndartmueller. Also the same can be said for the opposite side of the order here: https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L308