search

code-423n4 / 2022-03-sublime-findings

0 stars 0 forks source link

[WP-H9] `LenderPool.sol#start()` `startFeeFraction` can be used by a malicious/compromised owner to rug lenders #58

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L306-L330

Vulnerability details

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L306-L330

A configurable startFeeFraction with no upper bound can be claimed by the caller to a specified address.

The fee is not based on the gas cost, but on the _totalLent of the pool.

We believe this startFee reward is unnecessary and it creates a potential rug vector.

PoC

Given:

A malicious/compromised owner can first set the startFeeFraction to 0 and wait until the _startTime then do the following steps in one tx:

  1. Set startFeeFraction to 95%;
  2. Call start() and set to as the attacker's address;

As a result, 4.9M USDC will be stolen by the attacker.

Recommendation

  1. Consider removing the startFee rewards. The borrower should be the one who calls start() or the protocol can set up a keeper to call start().
  2. Or, at least add a reasonable upper bound when setting a new value for startFee, eg, 10 BPS or so.
ritik99 commented 2 years ago

The main issue arises here from the admin setting an absurdly high start fee maliciously. However, as mentioned in the contest readme, it is assumed that the admin is a trusted actor and thus would have no incentives to do so. The start fee has been introduced to act as an incentive for someone to call the start() function

HardlyDifficult commented 2 years ago

Agree - this vulnerability is focused on the owner account doing something unreasonable, but the readme states the assumption:

  1. The admin is a trusted actor

For the initial stages of the protocol development, the admin is going to be handled by us. Thus, contracts, functions, and critical thresholds are set to be upgradeable by the admin to allow effective action in case of emergencies. Over time, such functions would be decentralized.