search

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

5 stars 0 forks source link

Upgraded Q -> M from 417 [1657853118593] #443

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Judge has assessed an item in Issue #417 as Medium risk. The relevant finding follows:

HickupHH3 commented 2 years ago

(L4) Positions NFT mints aren't "safe" During a fill, two ERC721 tokens are minted (lines):

    // create long/short position for maker
    _mint(order.maker, uint256(orderHash));

    // create opposite long/short position for taker
    bytes32 oppositeOrderHash = hashOppositeOrder(order);
    positionId = uint256(oppositeOrderHash);
    _mint(msg.sender, positionId);

If the recipient is a contract, it may misbehave and lose the NFT, since it may expect a onERC721Received call.

Since order.maker can't be a contract (we need its signature), this only applies to msg.sender.

Recommendations Document that NFTs are minted this way, so contracts interacting with Putty can be aware. _safeMint is not suggested so we can save gas.

dup of #327