Closed code423n4 closed 1 year ago
How would you get the same tokenId?
tokenId = settings.totalSupply++;
I think the finding is invalid
You can front-run and close the auction, but the tokenId will not repeat.
Because the report didn't show how to get a repeated tokenId, I must close as invalid.
Please consider submitting a coded POC next time to avoid getting your report invalidated
Lines of code
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L161 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L90 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L204
Vulnerability details
Description
There is
createBid
function in theAuction
contract. The function accept the_tokenId
, which does not contain any information about the token itself. As a result, transactions of users can be front-runned to enforce user make a bid for the token with the same_tokenId
but for which he do not want to pay.Proof of Concept
Someone can send two transactions to the mempool:
settleCurrentAndCreateNewAuction
andcreateBid
with expected by them_tokenId
(see note section below). Then the malicious user can send a transaction with higher fee that creates an auction for a token which have the same_tokenId
but different content. This transaction most likely will be included in the block before the transaction from a non-malicious user.So, it is possible that the order of transactions will be the following:
settleCurrentAndCreateNewAuction
createBid
Note, that it is not necessarily for the user to send two transactions to mempool simultaneously to get under attack. That also can happen while chain reorg.
Recommended Mitigation Steps
It is better to use a hash of content as a unique identifier. Then it would be impossible to manipulate or front-run transactions to the auction.