search

sherlock-audit / 2022-11-isomorph-judging

5 stars 0 forks source link

hansfriese - The protocol shouldn't charge interests when paused #234

Open github-actions[bot] opened 1 year ago

github-actions[bot] commented 1 year ago

hansfriese

medium

The protocol shouldn't charge interests when paused

Summary

The protocol charges interest from users using virtualPrice and it increases when the protocol is paused.

As a result, users would be forced to pay more interests and experience an unexpected liquidation.

Vulnerability Detail

The protocol has 3 kinds of the vault and each one has pause/unpause option by pausers.

Also, each collateral would be paused using CollateralBook.pauseCollateralType().

But it updates the virtualPrice during the paused period and the below scenarios would be possible.

Scenario 1

  1. A user Alice opened a loan using some collaterals.
  2. The vault was paused for a while for some unexpected reason.
  3. Meanwhile, her loan was changed to a liquidatable one but she can't add collaterals(or close the loan) in the paused state.
  4. After the protocol is unpaused, she's trying to protect her loan by adding collaterals but Bob can liquidate her loan with front running.
  5. Even if her loan isn't liquidated, she should pay interests during the paused period and it's not fair for her.

Scenario 2

  1. A user Alice opened a loan with minOpeningMargin = 101%.
  2. After the protocol was paused for some reason, the admin decided to change minOpeningMargin = 105%.
  3. Alice wants to close her loan before it's applied because it's too high for her but she can't because it's paused.
  4. After the new minOpeningMargin is applied, Alice will be forced to pay interests of the higher minOpeningMargin for the paused period.

When I check other protocols to charge interests, it's normal to enable some ways to protect their loans during the paused period for users.

Currently, all functions don't work in the paused mode and it shouldn't charge interests in this case.

Impact

Users might be forced to pay more interests or their loans might be liquidated unexpectedly.

Code Snippet

https://github.com/sherlock-audit/2022-11-isomorph/blob/main/contracts/Isomorph/contracts/CollateralBook.sol#L127 https://github.com/sherlock-audit/2022-11-isomorph/blob/main/contracts/Isomorph/contracts/Vault_Base_ERC20.sol#L85 https://github.com/sherlock-audit/2022-11-isomorph/blob/main/contracts/Isomorph/contracts/Vault_Velo.sol#L141 https://github.com/sherlock-audit/2022-11-isomorph/blob/main/contracts/Isomorph/contracts/Vault_Base_ERC20.sol#L203-L221 https://github.com/sherlock-audit/2022-11-isomorph/blob/main/contracts/Isomorph/contracts/Vault_Velo.sol#L248-L265

Tool used

Manual Review

Recommendation

We shouldn't increase the virtualPrice during the paused period.

kree-dotcom commented 1 year ago

This was a design decision but we have decided to change it so that users can closeLoans, addCollateral or be liquidated when a collateral/vault is Paused.

Fixed here https://github.com/kree-dotcom/isomorph/commit/be279b394931393feaa119bc2ebe7d6bfeb575ba Collateral pausing has been modified to match Vault pausing. Because a user can closeLoan, addCollateral or be liquidated while a collateral is paused there is no reason to freeze interest accruing on the collateral. Like a paused Vault, the only action not possible when a collateral is paused is openLoan()

IAm0x52 commented 1 year ago

Fixes look good. Reverting another fix, allowing interest to accrue while paused