`usedFunds` can be greater than `totalFunds` in contract `KangarooVault`, which leads to `KangarooVault` being unable to close its trades and users being unable to withdraw.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-polynomial/blob/main/src/KangarooVault.sol#L792 https://github.com/code-423n4/2023-03-polynomial/blob/main/src/KangarooVault.sol#L279

Vulnerability details

Impact

KangarooVault contract can't close the trades and users can't withdraw from it, then users and KangarooVaults will lose a lot of funds.

Proof of concept

1. In contract KangarooVault, usedFunds is the uint256 variable which tracks the funds utilitized from the vault. And totalFunds is the uin256 variable which tracks the funds claimed by vault from profits and users' depositing.
2. Contract KangarooVault has no check if totalFunds >= usedFunds when usedFunds is increased (transfer from vault) or totalFunds is decreased (transfer to vault).
3. If someone transfers funds directly into the vault, usedFunds can be greater than totalFunds because the vault can transfer out more than totalFunds.
4. When usedFunds > totalFunds, KangarooVault can not close its trades because it will revert on underflow in function _resetTrade:

   function _resetTrade() internal {
       ...
       totalFunds -= usedFunds;
       ...
   }

5. When usedFunds > totalFunds, user can't not withdraw by function processWithdrawalQueue because it will revert on underflow.

   function processWithdrawalQueue(uint256 idCount) external nonReentrant {
   for (uint256 i = 0; i < idCount; i++) {
       ...
   
       uint256 availableFunds = totalFunds - usedFunds;
   
       ...

Scenerio:

• A deposit 1000 SUSD, then totalFunds = 1000 SUSD, usedFunds = 0
• B transfer directly 1000 SUSD to the KangarooVault, that doesn't change totalFunds and usedFunds
• KangarooVault opens a short position and uses 2000 SUSD to increase the margin, then usedFunds = 2000
• After that, usedFunds > totalFunds (2000 > 1000) then KangarooVault can't close its position

Tool used

Manual Review

Recommended Mitigation Steps

Should add the checks if totalFunds >= usedFunds when increasing usedfunds or decreasing totalFunds in contract KangarooVault.sol

[c4-sponsor]

mubaris marked the issue as disagree with severity

[c4-sponsor]

mubaris marked the issue as sponsor acknowledged

[c4-judge]

JustDravee changed the severity to 2 (Med Risk)

[c4-judge]

JustDravee marked the issue as satisfactory

[c4-judge]

JustDravee marked the issue as selected for report