hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

LP token lending protocol
MIT License
2 stars 1 forks source link

**The current implementation of `calculateUserAccountData` will freeze assets and disable liquidations** #4

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @https://github.com/maarcweiss Submission hash (on-chain): 0x6bddb0c819575a51ced25c0fffb628214677f641f66c532e0c3b43e5a6b7d969 Severity: high severity

Description: The current implementation of calculateUserAccountData will freeze assets and disable liquidations

The current implementation of calculateUserAccountData will freeze assets and disable liquidations because calculateUserAccountData does check assetMappings.getParams(vars.currentReserveAddress); for every asset used as the collateral for any borrow. Which interferes with the liquidation process.

https://github.com/VMEX-finance/vmex/blob/b0dc00c5dd6bdcac05827128d14dcdc730f19e1c/packages/contracts/contracts/protocol/libraries/logic/GenericLogic.sol#L222-L228

This: assetMappings.getParams(vars.currentReserveAddress); does check that each asset used by the borrower is allowed assetMappings[asset].isAllowed = true; Therefore, if an asset is taken out from the "allowlist", liquidations for that user/s will be disabled.

PoC

Steps for the attack to happen:

SEVERITY

High. Liquidations will be frozen for un whitelisted assets and existing positions

A LINK TO THE GITHUB ISSUE

https://github.com/VMEX-finance/vmex/blob/b0dc00c5dd6bdcac05827128d14dcdc730f19e1c/packages/contracts/contracts/protocol/libraries/logic/GenericLogic.sol#L181

https://github.com/VMEX-finance/vmex/blob/b0dc00c5dd6bdcac05827128d14dcdc730f19e1c/packages/contracts/contracts/protocol/libraries/logic/GenericLogic.sol#L222-L228

SOLUTION

Once an asset is in the system, being disabled should not be affected, only for new assets. If not stuck funds will be a cause of the wrong design.

ksyao2002 commented 1 year ago

The protocol already prevents setting isAllowed to false if there is already open borrowing or collateral positions. Please check L59 of AssetMappings.sol for the validation logic. This validation is called in L296 of AssetMappings.sol, which is the guard for setting asset allowed or not.