From 432584e9c9a437f6be580fa8543750fc7f4fe794 Mon Sep 17 00:00:00 2001 From: Aodhgan Date: Tue, 29 Jun 2021 11:57:10 -0700 Subject: [PATCH] use safe transferFrom for ERC721 --- contracts/prize-pool/PrizePool.sol | 13 ++++++++++++- test/PrizePool.test.js | 9 +++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/contracts/prize-pool/PrizePool.sol b/contracts/prize-pool/PrizePool.sol index 372fb89a..6512ce83 100644 --- a/contracts/prize-pool/PrizePool.sol +++ b/contracts/prize-pool/PrizePool.sol @@ -27,6 +27,7 @@ abstract contract PrizePool is PrizePoolInterface, OwnableUpgradeable, Reentranc using SafeMathUpgradeable for uint256; using SafeCastUpgradeable for uint256; using SafeERC20Upgradeable for IERC20Upgradeable; + using SafeERC20Upgradeable for IERC721Upgradeable; using MappedSinglyLinkedList for MappedSinglyLinkedList.Mapping; using ERC165CheckerUpgradeable for address; @@ -159,6 +160,10 @@ abstract contract PrizePool is PrizePoolInterface, OwnableUpgradeable, Reentranc uint256 amount ); + /// @dev Emitted when there was an error thrown awarding an External ERC721 + event ErrorAwardingExternalERC721(bytes error); + + struct CreditPlan { uint128 creditLimitMantissa; uint128 creditRateMantissa; @@ -599,7 +604,13 @@ abstract contract PrizePool is PrizePoolInterface, OwnableUpgradeable, Reentranc } for (uint256 i = 0; i < tokenIds.length; i++) { - IERC721Upgradeable(externalToken).transferFrom(address(this), to, tokenIds[i]); + try IERC721Upgradeable(externalToken).safeTransferFrom(address(this), to, tokenIds[i]){ + + } + catch(bytes memory error){ + emit ErrorAwardingExternalERC721(error); + } + } emit AwardedExternalERC721(to, externalToken, tokenIds); diff --git a/test/PrizePool.test.js b/test/PrizePool.test.js index 19735002..72a1dc35 100644 --- a/test/PrizePool.test.js +++ b/test/PrizePool.test.js @@ -962,5 +962,14 @@ describe('PrizePool', function() { .to.emit(prizePool, 'AwardedExternalERC721') .withArgs(wallet.address, erc721token.address, [NFT_TOKEN_ID]) }) + + it('should not DoS with faulty ERC721s', async () => { + + await yieldSourceStub.mock.canAwardExternal.withArgs(erc721token.address).returns(true) + await erc721token.mock.transferFrom.withArgs(prizePool.address, wallet.address, NFT_TOKEN_ID).reverts() + + await expect(prizeStrategy.call(prizePool, 'awardExternalERC721', wallet.address, erc721token.address, [NFT_TOKEN_ID])) + .to.emit(prizePool, 'ErrorAwardingExternalERC721') + }) }) });