From f61365a4f05effd5145a299ac874e8f0950b4e27 Mon Sep 17 00:00:00 2001 From: Eduardo Melo Date: Tue, 13 Aug 2024 20:19:51 -0300 Subject: [PATCH] fix: changing attack contract --- contracts/mocks/ReentrancyAttack.sol | 30 +++++++++++++++---- test/OriumSftMarketplace.test.ts | 44 +++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/contracts/mocks/ReentrancyAttack.sol b/contracts/mocks/ReentrancyAttack.sol index 388dbe2..fbe13ea 100644 --- a/contracts/mocks/ReentrancyAttack.sol +++ b/contracts/mocks/ReentrancyAttack.sol @@ -5,17 +5,35 @@ import '../OriumSftMarketplace.sol'; contract ReentrancyAttack { OriumSftMarketplace public marketplace; + RentalOffer public offer; + uint64 public duration; + bool public reentered = false; constructor(OriumSftMarketplace _marketplace) { marketplace = _marketplace; } - receive() external payable {} + receive() external payable { + if (!reentered && address(marketplace).balance > 0) { + reentered = true; + // Try to re-enter the marketplace + marketplace.acceptRentalOffer{ value: msg.value / 2 }(offer, duration); + } + } + + function attack(RentalOffer calldata _offer, uint64 _duration) external payable { + offer = _offer; + duration = _duration; + + marketplace.acceptRentalOffer{ value: msg.value }(_offer, _duration); + } + + function attackWithRecursiveCalls(RentalOffer calldata _offer, uint64 _duration, uint times) external payable { + offer = _offer; + duration = _duration; - function attemptDoubleAccept(RentalOffer calldata _offer, uint64 _duration) external payable { - // First accept call - marketplace.acceptRentalOffer{ value: msg.value / 2 }(_offer, _duration); - // Second accept call in the same transaction - marketplace.acceptRentalOffer{ value: msg.value / 2 }(_offer, _duration); + for (uint i = 0; i < times; i++) { + marketplace.acceptRentalOffer{ value: msg.value / times }(_offer, _duration); + } } } diff --git a/test/OriumSftMarketplace.test.ts b/test/OriumSftMarketplace.test.ts index c93b0a5..8be9701 100644 --- a/test/OriumSftMarketplace.test.ts +++ b/test/OriumSftMarketplace.test.ts @@ -723,7 +723,38 @@ describe('OriumSftMarketplace', () => { ).to.be.revertedWith('OriumSftMarketplace: Insufficient native token amount') }) - it('should prevent double accept in the same transaction', async () => { + it('should detect reentrancy attack during fee transfer', async () => { + const AttackContract = await ethers.getContractFactory('ReentrancyAttack') + attackContract = (await AttackContract.deploy(marketplace)) as ReentrancyAttack + await attackContract.waitForDeployment() + + await marketplaceRoyalties + .connect(operator) + .setTrustedFeeTokenForToken([rentalOffer.tokenAddress], [AddressZero], [true]) + + rentalOffer.feeTokenAddress = AddressZero + rentalOffer.feeAmountPerSecond = toWei('0.0000001') + const totalFeeAmount = rentalOffer.feeAmountPerSecond * BigInt(duration) + rentalOffer.nonce = `0x${randomBytes(32).toString('hex')}` + await marketplace.connect(lender).createRentalOffer({ ...rentalOffer, commitmentId: BigInt(0) }) + rentalOffer.commitmentId = BigInt(2) + + // Attempt the attack + try { + await attackContract.attack(rentalOffer, duration, { value: totalFeeAmount }) + console.log('Reentrancy attack did not revert the transaction.') + } catch (error: any) { + if ( + error.message.includes('OriumSftMarketplace: Reentrancy detected or insufficient native token amount') + ) { + console.log('Reentrancy was correctly detected.') + } else { + console.log('The transaction failed for another reason: ', error.message) + } + } + }) + + it('should revert on multiple reentrant calls', async () => { const AttackContract = await ethers.getContractFactory('ReentrancyAttack') attackContract = (await AttackContract.deploy(marketplace)) as ReentrancyAttack await attackContract.waitForDeployment() @@ -741,16 +772,13 @@ describe('OriumSftMarketplace', () => { await borrower.sendTransaction({ to: attackContract.getAddress(), - value: totalFeeAmount, + value: totalFeeAmount * BigInt(6), }) - const doubleTotalFeeAmount = totalFeeAmount * BigInt(2) - + // Attempt the attack await expect( - attackContract - .connect(borrower) - .attemptDoubleAccept(rentalOffer, duration, { value: doubleTotalFeeAmount.toString() }), - ).to.be.revertedWith('OriumSftMarketplace: This offer has an ongoing rental') + attackContract.attackWithRecursiveCalls(rentalOffer, duration, 5, { value: totalFeeAmount }), + ).to.be.revertedWith('OriumSftMarketplace: Insufficient native token amount') }) describe('Fees', async function () {