search

code-423n4 / 2021-10-pooltogether-findings

0 stars 0 forks source link

`PrizePool.awardExternalERC721()` Erroneously Emits Events #62

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

leastwood

Vulnerability details

Impact

The awardExternalERC721() function uses solidity's try and catch statement to ensure a single tokenId cannot deny function execution. If the try statement fails, an ErrorAwardingExternalERC721 event is emitted with the relevant error, however, the failed tokenId is not removed from the list of tokenIds emitted at the end of function execution. As a result, the AwardedExternalERC721 is emitted with the entire list of tokenIds, regardless of failure. An off-chain script or user could therefore be tricked into thinking an ERC721 tokenId was successfully awarded.

Proof of Concept

https://github.com/pooltogether/v4-core/blob/master/contracts/prize-pool/PrizePool.sol#L250-L270

Tools Used

Manual code review

Recommended Mitigation Steps

Consider emitting only successfully transferred tokenIds in the AwardedExternalERC721 event.

PierrickGT commented 3 years ago

PR: https://github.com/pooltogether/v4-core/pull/246

GalloDaSballo commented 3 years ago

The sponsor acknowledged and mitigated by actively managing a list of _awardedTokenIds to keep track of the tokens that didn't go through the catch part of the error hadling