Skip to content

Latest commit

 

History

History
99 lines (66 loc) · 4.01 KB

071.md

File metadata and controls

99 lines (66 loc) · 4.01 KB

Calm Fiery Llama

Medium

Updating the unhealthyResponsePeriod also impacts past vouches

Summary

Currently, a voucher can decide whether to mark a vouch as unhealthy, as long as the unhealthyResponsePeriod has not expired since they unvouched. However, users may still be able to mark a vouch as unhealthy even after the unhealthyResponsePeriod has already passed before, if the unhealthyResponsePeriod is extended.

Root Cause

When it is checked whether the author still has time to mark a vouch as unhealthy, the current unhealthyResponsePeriod is used instead of the one that was in effect when the user unvouched.

Internal pre-conditions

None.

External pre-conditions

None.

Attack Path

  1. Alice vouches for ProfileA.
  2. Alice calls EthosVouch::unvouch() to revoke her vouch because she has lost trust in the profile she previously vouched for. She can mark the vouch as unhealthy until the unhealthyResponsePeriod (currently set to 24 hours) expires.
  3. The unhealthyResponsePeriod expires.
  4. Twelve hours later, updateUnhealthyResponsePeriod() is called to increase the unhealthy response period to 48 hours.
  5. Now, Alice can call EthosVouch::markUnhealthy(), even though the unhealthyResponsePeriod has already expired once.

Impact

Users will still be able to mark a vouch as unhealthy, even if the unhealthyResponsePeriod at the time of unvouching has already expired. This means that even vouches that have been unvouched for several months could be marked as unhealthy if the unhealthyResponsePeriod was extended long enough.

As a result, vouchers can mark a vouch as unhealthy, even if they did not distrust the subject when they unvouched it. This increases the risk of manipulation of the protocol's reputation system.

PoC

The following test should be added in EthosVouch.test.ts:

    it('should succeed marking vouch unhealthy when unhealthyResponsePeriod extended', async () => {
      const {
        ethosProfile,
        ethosVouch,
        ADMIN,
        OWNER,
        PROFILE_CREATOR_0,
        VOUCHER_0,
      } = await loadFixture(deployFixture);
  
      await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_0.address);
      await ethosProfile.connect(PROFILE_CREATOR_0).createProfile(1);

      await ethosProfile.connect(OWNER).inviteAddress(VOUCHER_0.address);
      await ethosProfile.connect(VOUCHER_0).createProfile(1);
  
      expect(await ethosProfile.profileIdByAddress(PROFILE_CREATOR_0.address)).to.be.equal(
        2,
        'wrong profile Id',
      );
        
      await ethosVouch.connect(VOUCHER_0).vouchByProfileId(2, DEFAULT_COMMENT, DEFAULT_METADATA, {
        value: ethers.parseEther('1.1'),
      });

      expect(await ethosVouch.vouchCount()).to.equal(1, 'Wrong vouchCount');

      await ethosVouch.connect(ADMIN).updateUnhealthyResponsePeriod(86400);
      expect(await ethosVouch.unhealthyResponsePeriod()).to.equal(
        86400,
        'Wrong unhealthyResponsePeriod, 0',
      );

      await ethosVouch.connect(VOUCHER_0).unvouch(0);

      await time.increase(100);

      await time.increase(await ethosVouch.unhealthyResponsePeriod());

      await expect(ethosVouch.connect(VOUCHER_0).markUnhealthy(0))
        .to.be.revertedWithCustomError(ethosVouch, 'CannotMarkVouchAsUnhealthy')
        .withArgs(0);

      await ethosVouch.connect(ADMIN).updateUnhealthyResponsePeriod(172800);
      expect(await ethosVouch.unhealthyResponsePeriod()).to.equal(
        172800,
        'Wrong unhealthyResponsePeriod, 1',
      );

      await time.increase(43100);

      await ethosVouch.connect(VOUCHER_0).markUnhealthy(0)
    });

Mitigation

Only the unhealthyResponsePeriod at the time a user unvouches should be considered when EthosVouch::markUnhealthy() is called, so that once this period expires, the user will no longer be able to mark a vouch as unhealthy.