code-423n4 / 2023-05-venus-findings

2 stars 1 forks source link

Wrong blocksPerYear calculation in WhitePaperInterestRateModel.sol #559

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/WhitePaperInterestRateModel.sol#L17

Vulnerability details

Impact

In WhitePaperInterestRateModel.sol,

File: contracts/WhitePaperInterestRateModel.sol

17   uint256 public constant blocksPerYear = 2102400;

There is wrong calculation of blocksPerYear and blocksPerYear is the approximate number of blocks per year that is assumed by the interest rate model.

The contracts will be deployed on BNB Chain and in a Binance smart chain, the blocks are made every 3 seconds. and It was also understood the blockPerYear calculation with sponsers team and it was confirmed that 3 seconds are also considered in calculations for BaseJumpRateModelV2.sol

This can also confirmed by this reference

Now the issue is WhitePaperInterestRateModel.sol is taken from compound protocol smart contract which is an Ethereum smart contract.. Therefore being deployed on Ethereum blockchain, the Ethereum Average Block Time is 12 to 12.25 seconds

Calculations-

1)Compound blocksPerYear calculation in WhitePaperInterestRateModel.sol as shown below,

Ethereum block time considered = 15 seconds

Number of seconds in a year = 365 24 60 * 60 = 3,15,36,000 seconds

blocksPerYear = Number of seconds in a year / Ethereum block time considered = 3,15,36,000 / 15 = 21,02,400 blocks

which also matches with blocksPerYear considered in compound WhitePaperInterestRateModel as shown below,

File: contracts/WhitePaperInterestRateModel.sol

19    uint public constant blocksPerYear = 2102400;

2)Since the smart contracts will be deployed on BNB chain. The blocksPerYear calculations must also be as per Binance smart chain block time. Which should be as follows,

Binance block time considered = 3 seconds Number of seconds in a year = 365 24 60 * 60 = 31536000 seconds

blocksPerYear = Number of seconds in a year / Ethereum block time considered = 31536000 / 3 = 10512,000 blocks

If the blocksPerYear is wrongly calculated then baseRatePerBlock and multiplierPerBlock will also be incorrect,

File: contracts/WhitePaperInterestRateModel.sol

36    constructor(uint256 baseRatePerYear, uint256 multiplierPerYear) {
37        baseRatePerBlock = baseRatePerYear / blocksPerYear;
38        multiplierPerBlock = multiplierPerYear / blocksPerYear;
39
40        emit NewInterestParams(baseRatePerBlock, multiplierPerBlock);
41    }

baseRatePerYear is the approximate target base APR and multiplierPerYear The rate of increase in interest rate wrt utilization. Also baseRatePerYear and multiplierPerYear are immutable variables therefore any wrong calculation in baseRatePerYear and multiplierPerYear due to blocksPerYear will cause redeployment of contract will be expensive with a potentail risk of reputation damage.

getBorrowRate( ) which calculates the current borrow rate per block, with the error code expected by the market will also be incorrect.

and getSupplyRate( ) which calculates the current supply rate per block supply rate per block will also be incorrect.

Proof of Concept

Link to code

Tools Used

Manaual review

Recommended Mitigation Steps

Check above Impact section where the blocksPerYear for BNB chain calculation is explained and do the modification in code as below,

File: contracts/WhitePaperInterestRateModel.sol

-    uint256 public constant blocksPerYear = 2102400;
+    uint256 public constant blocksPerYear = 10512000;

Assessed type

Other

c4-judge commented 1 year ago

0xean marked the issue as primary issue

c4-sponsor commented 1 year ago

chechu marked the issue as sponsor confirmed

c4-judge commented 1 year ago

0xean marked the issue as satisfactory

c4-judge commented 1 year ago

0xean changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

0xean changed the severity to 3 (High Risk)

c4-judge commented 1 year ago

0xean marked issue #320 as primary and marked this issue as a duplicate of 320