Skip to content
This repository has been archived by the owner on Nov 19, 2023. It is now read-only.

xiaoming90 - Vault account might not be able to exit after liquidation #192

Open
sherlock-admin opened this issue May 15, 2023 · 2 comments
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

xiaoming90

high

Vault account might not be able to exit after liquidation

Summary

The vault exit might fail after a liquidation event, leading to users being unable to main their positions.

Vulnerability Detail

Assume that a large portion of the vault account gets liquidated which results in a large amount of cash deposited into the vault account's cash balance. In addition, interest will also start accruing within the vault account's cash balance.

Let $x$ be the primaryCash of a vault account after a liquidation event and interest accrual.

The owner of the vault account decided to exit the vault by calling exitVault. Within the exitVault function, the vaultAccount.tempCashBalance will be set to $x$.

Next, the lendToExitVault function is called. Assume that the cost in prime cash terms to lend an offsetting fCash position is $-y$ (primeCashCostToLend). The updateAccountDebt function will be called, and the vaultAccount.tempCashBalance will be updated to $x + (-y) \Rightarrow x - y$. If $x > y$, then the new vaultAccount.tempCashBalance will be more than zero.

Subsequently, the redeemWithDebtRepayment function will be called. However, since vaultAccount.tempCashBalance is larger than zero, the transaction will revert, and the owner cannot exit the vault.

https://github.com/sherlock-audit/2023-03-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultConfiguration.sol#L429

File: VaultConfiguration.sol
424:             if (vaultAccount.tempCashBalance < 0) {
425:                 int256 x = vaultConfig.primeRate.convertToUnderlying(vaultAccount.tempCashBalance).neg();
426:                 underlyingExternalToRepay = underlyingToken.convertToUnderlyingExternalWithAdjustment(x).toUint();
427:             } else {
428:                 // Otherwise require that cash balance is zero. Cannot have a positive cash balance in this method
429:                 require(vaultAccount.tempCashBalance == 0);
430:             }

Impact

The owner of the vault account would not be able to exit the vault to main their position. As such, their assets are stuck within the protocol.

Code Snippet

https://github.com/sherlock-audit/2023-03-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultConfiguration.sol#L429

Tool used

Manual Review

Recommendation

Consider refunding the excess positive vaultAccount.tempCashBalance to the users so that vaultAccount.tempCashBalance will be cleared (set to zero) before calling the redeemWithDebtRepayment function.

@github-actions github-actions bot added the Medium A valid Medium severity issue label May 19, 2023
@T-Woodward T-Woodward added the Sponsor Confirmed The sponsor acknowledged this issue is valid label May 21, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label May 29, 2023
@jeffywu
Copy link

jeffywu commented Jul 11, 2023

Fixed in: notional-finance/contracts-v2#133

@jeffywu jeffywu added the Will Fix The sponsor confirmed this issue will be fixed label Jul 11, 2023
@xiaoming9090
Copy link
Collaborator

@0xleastwood + @xiaoming9090: Verified. Fixed in PR notional-finance/contracts-v2#133

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants