Submitted by 0x52, also found by Dravee, c7e7eff, xiaoming90, KingNFT, Big0XDev, and cccz
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/ui/WPunkGateway.sol#L77-L95
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/ui/WPunkGateway.sol#L129-L155
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/ui/WPunkGateway.sol#L167-L193
All CryptoPunk deposits can be stolen.
CryptoPunks were created before the ERC721 standard. A consequence of this is that they do not possess the transferFrom method. To approximate this a user can offerPunkForSaleToAddress for a price of 0 to effectively approve the contract to transferFrom.
WPunkGateway.sol#L77-L95
The current implementation of WPunkGateway#supplyPunk allows anyone to execute and determine where the nTokens are minted to. To complete the flow supply flow a user would need to offerPunkForSaleToAddress for a price of 0 to WPunkGateway. After they have done this, anyone can call the function to deposit the punk and mint the nTokens to themselves, effectively stealing it.
Example:
User A owns tokenID of 1. They want to deposit it so they call offerPunkForSaleToAddress with an amount of 0, effectively approving the WPunkGateway to transfer their CryptoPunk. User B monitors the transactions and immediately calls supplyPunk with themselves as onBehalfOf. This completes the transfer of the CryptoPunk and deposits it into the pool but mints the nTokens to User B, allowing them to effectively steal the CryptoPunk.
The same fundamental issue exists with acceptBidWithCredit and batchAcceptBidWithCredit.
Query the punkIndexToAddress to find the owner and only allow owner to deposit:
yubo-ruan (Paraspace) confirmed
