search

sherlock-audit / 2023-06-unstoppable-judging

0 stars 0 forks source link

stopthecap - Debt is not updated when removing margin from a position #9

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

stopthecap

high

Debt is not updated when removing margin from a position

Summary

Debt is not updated when removing margin from a position

Vulnerability Detail

Traders are allowed to remove margin/collateral from their positions: https://github.com/sherlock-audit/2023-06-unstoppable/blob/94a68e49971bc6942c75da76720f7170d46c0150/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L528-L546

as you can see, the position can only have collateral removed if it is not liquidable: https://github.com/sherlock-audit/2023-06-unstoppable/blob/94a68e49971bc6942c75da76720f7170d46c0150/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L487

The problem is that the total total_debt_shares[_debt_token] is not updated when checking whether the positon is liquidable neither when calling remove_margin You can see on the following links the progression of the function calls to calculate whether a position is liquidable or not:

https://github.com/sherlock-audit/2023-06-unstoppable/blob/94a68e49971bc6942c75da76720f7170d46c0150/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L444

https://github.com/sherlock-audit/2023-06-unstoppable/blob/94a68e49971bc6942c75da76720f7170d46c0150/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L1142

https://github.com/sherlock-audit/2023-06-unstoppable/blob/94a68e49971bc6942c75da76720f7170d46c0150/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L1099

https://github.com/sherlock-audit/2023-06-unstoppable/blob/94a68e49971bc6942c75da76720f7170d46c0150/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L1111

https://github.com/sherlock-audit/2023-06-unstoppable/blob/94a68e49971bc6942c75da76720f7170d46c0150/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L1117C16-L1117C16

As seen, total_debt_shares[_debt_token] is not updated to the current total_debt_shares[_debt_token] by calling _update_debt and therefore a stale total_debt_shares[_debt_token] is used to calculate whether a position is liquidable. Allowing positions that are in fact liquidable remove margin because the debt is not updated.

Impact

Not updating the debt before removing collateral (margin) from your position does allow a trader to remove collateral from a liquidable position

Code Snippet

https://github.com/sherlock-audit/2023-06-unstoppable/blob/94a68e49971bc6942c75da76720f7170d46c0150/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L528-L546

https://github.com/sherlock-audit/2023-06-unstoppable/blob/94a68e49971bc6942c75da76720f7170d46c0150/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L1117C16-L1117C16

Tool used

Manual Review

Recommendation

Call _update_debt at the beginning of the remove margin function

Unstoppable-DeFi commented 1 year ago

https://github.com/Unstoppable-DeFi/unstoppable-dex-audit/pull/1

maarcweiss commented 1 year ago

Fixed by calling update debt when removing margin from positions