Skip to content

Latest commit

 

History

History
64 lines (44 loc) · 3.4 KB

083.md

File metadata and controls

64 lines (44 loc) · 3.4 KB

Fast Khaki Raccoon

Medium

Pairs will have less available assets to withdraw from the vault due to incorrect implementation

Summary

Pairs will have less available assets to withdraw from the vault due to incorrect implementation

Root Cause

The assets available for a vault are computed using this function:

function totalAvailableAssetsForVault(address _vault) public view override returns (uint256 _totalVaultAvailable) {
        uint256 _overallAvailable = totalAvailableAssets();

        _totalVaultAvailable = vaultMaxAllocation[_vault] > vaultDeposits[_vault] ? vaultMaxAllocation[_vault] - vaultDeposits[_vault] : 0;

        _totalVaultAvailable = _overallAvailable < _totalVaultAvailable ? _overallAvailable : _totalVaultAvailable;
    }

The issue is that the vaultDeposits value for a particular vault can be higher than the actual value, resulting in less available assets. This is because upon updating the vault metadata, we update these 3 values:

vaultUtilization[_vault] = ...;
_totalAssetsUtilized = _totalAssetsUtilized - _currentAssetsUtilized + vaultUtilization[_vault];
_totalAssets = _totalAssets - _currentAssetsUtilized + vaultUtilization[_vault];

Then, upon a vault returning the borrowed assets, we have this code:

vaultDeposits[_vault] -= _assetAmt > vaultDeposits[_vault] ? vaultDeposits[_vault] : _assetAmt;
vaultUtilization[_vault] -= _assetAmt;
_totalAssetsUtilized -= _assetAmt;

This code is correct if the CBR has went up as the vault deposits will be less than the vault utilization, thus they will simply cap at the vault deposits value and go to 0 due to the code used in the vaultDeposits line above. However, it is incorrect if the CBR went down due to a reason like bad debt liquidation in the underlying vault. In that case, vault utilization can go to 0 while the vault deposits can stay at a value above 0 even though there are actually no deposits.

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

  1. The LendingAssetVault has 100 assets and 100 shares, 100 of those assets are utilized and there are also 100 vault deposits (very simple scenario)
  2. Bad debt liquidation occurs and upon updating the metadata, the assets go to 50 and the utilized assets go to 50 as well, vault deposits stay at 100
  3. LendingAssetVault::whitelistWithdraw() is called which brings the utilization and total assets to 0 while the vault deposits go to 50
  4. LendingAssetVault::totalAvailableAssetsForVault() will now return 50 less assets than the actual amount as there are not actually any deposits into the vault

Impact

Available assets for a vault will go down and down overtime on each bad debt liquidation, resulting in the users who have supplied to it to have their assets not providing any value, thus less interest as they won't be utilized which results in loss of funds for those suppliers. Also, FraxlendPair will not function properly as it can not utilize assets that are supposed to be utilizable.

PoC

No response

Mitigation

2 options:

  • if the utilized assets of a vault go to 0, also zero out the vault deposits
  • remove the vault deposits mapping altogether as they simply add another weird thing to handle, their only use case is the available assets so simply remove them and think of a different way to compute the available assets based on the other values that are already in the code