From 6d0d99ff2748e5467a0b995e97ea0738f0e55af0 Mon Sep 17 00:00:00 2001 From: Eduardo Melo Date: Tue, 13 Aug 2024 16:33:54 -0300 Subject: [PATCH] feat: including ReentrancyAttack contract to test if the offer can be accept twice in the same tx --- contracts/mocks/ReentrancyAttack.sol | 21 ++++++++++++++++++ test/OriumSftMarketplace.test.ts | 32 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 contracts/mocks/ReentrancyAttack.sol diff --git a/contracts/mocks/ReentrancyAttack.sol b/contracts/mocks/ReentrancyAttack.sol new file mode 100644 index 0000000..388dbe2 --- /dev/null +++ b/contracts/mocks/ReentrancyAttack.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.9; + +import '../OriumSftMarketplace.sol'; + +contract ReentrancyAttack { + OriumSftMarketplace public marketplace; + + constructor(OriumSftMarketplace _marketplace) { + marketplace = _marketplace; + } + + receive() external payable {} + + 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); + } +} diff --git a/test/OriumSftMarketplace.test.ts b/test/OriumSftMarketplace.test.ts index 8cf7cdc..c93b0a5 100644 --- a/test/OriumSftMarketplace.test.ts +++ b/test/OriumSftMarketplace.test.ts @@ -16,6 +16,7 @@ import { OriumSftMarketplace, SftRolesRegistrySingleRole, SftRolesRegistrySingleRoleLegacy, + ReentrancyAttack, } from '../typechain-types' describe('OriumSftMarketplace', () => { @@ -27,6 +28,7 @@ describe('OriumSftMarketplace', () => { let secondMockERC1155: MockERC1155 let wearableToken: MockERC1155 let mockERC20: MockERC20 + let attackContract: ReentrancyAttack // We are disabling this rule because hardhat uses first account as deployer by default, and we are separating deployer and operator // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -721,6 +723,36 @@ describe('OriumSftMarketplace', () => { ).to.be.revertedWith('OriumSftMarketplace: Insufficient native token amount') }) + it('should prevent double accept in the same transaction', 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) + + await borrower.sendTransaction({ + to: attackContract.getAddress(), + value: totalFeeAmount, + }) + + const doubleTotalFeeAmount = totalFeeAmount * BigInt(2) + + await expect( + attackContract + .connect(borrower) + .attemptDoubleAccept(rentalOffer, duration, { value: doubleTotalFeeAmount.toString() }), + ).to.be.revertedWith('OriumSftMarketplace: This offer has an ongoing rental') + }) + describe('Fees', async function () { const feeAmountPerSecond = toWei('1') const feeAmount = feeAmountPerSecond * BigInt(duration)