sherlock-audit / 2024-05-elfi-protocol-judging

11 stars 7 forks source link

KupiaSec - The `AssetsProcess.withdraw()` function doesn't update the `CommonData` #209

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

KupiaSec

High

The AssetsProcess.withdraw() function doesn't update the CommonData

Summary

The withdraw() function does not modify the tradeCollateralTokenDatas property in the CommonData.

Vulnerability Detail

Withdrawing reduces the total collateral. Therefore, the withdraw() function should decrease the tradeCollateralTokenDatas[token].totalCollateral in the CommonData accordingly. However, this logic is currently missing. As a result, the value of tradeCollateralTokenDatas[token].totalCollateral will exceed the actual collateral amount, leading to smaller available deposit amounts than it should be. Since the tradeCollateralTokenDatas[token].totalCollateral value never decreases, it could reach the collateralTotalCap, making future deposits impossible.

Impact

The available deposit amount becomes smaller than it should be. Additionally, tradeCollateralTokenDatas[token].totalCollateral could reach the collateralTotalCap, resulting in deposits becoming impossible.

Code Snippet

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/main/elfi-perp-contracts/contracts/process/AssetsProcess.sol#L122-L155

Tool used

Manual Review

Recommendation

It is recommended to update the the CommonData accordingly.

+       CommonData.load().subTradeTokenCollateral(token, params.amount);

Duplicate of #113