Non view function is called with staticcall in `CErc20Delegator`

[code423n4]

Lines of code

https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/CErc20Delegator.sol#L237 https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/CErc20Delegator.sol#L246

Vulnerability details

Impact

When using CToken implementation with CErc20Delegator, the functions borrowRatePerBlock and supplyRatePerBlock will revert when the underlying functions try to update some states.

Detail

The v1 of borrowRatePerBlock and supplyRatePerBlock were view functions, but they are not anymore. The CErc20Delegator is still using delegateToViewImplementation for those functions. Those functions can be used, as long as the implementation does not update any state variables, i.e. the block number increase since the last update is less or equal to the updateFrequency. However, when these functions are called after sufficient blocks are mined, they are going to revert. Although one can still call the implementation using delegateToImplementation, it is not a good usability, especially if those functions are used for external user interface.

Proof Of Concept

gist for the test

The gist shows a simple test. It calls borrowRatePerBlock and supplyRatePerBlock first time, it suceeds. Then, it mines for more than 300 times, which is the updateFrequency parameter. Then it calls again then fails.

Notes on the test file:

• The setup is taken from tests/Treasury/Accountant.test.ts

• using solidity from ethereum-waffle for chai to use reverted

  // in hardhat.config.js
  import chai from "chai";
  import { solidity } from "ethereum-waffle";
  
  chai.use(solidity);

Tools Used

hardhat

Recommended Mitigation Steps

Instead of using delegateToViewImplementation use delegateToImplementation. Alternatively, implement view functions to query these rates in NoteInterest.sol and CToken.sol. It will enable to query the rates without spending gas.

[GalloDaSballo]

The warden has shown how, due to the usage of delegateToViewImplementation the function call to borrowRatePerBlock and supplyRatePerBlock will revert, provided some time has passed.

There is no more sever loss of availability than a function that doesn't work, however, at this time, because the functions are view and "niche", the actual underlying functionality is still available via delegateToViewImplementation, and the internal accounting of the protocol is still functioning, I believe Medium Severity to be appropriate.