fix: changing attack contract

OriumNetwork · Aug 13, 2024 · f61365a · f61365a
1 parent 6d0d99f
commit f61365a
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 14 deletions.
diff --git 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
@@ -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 () {