search

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

0 stars 0 forks source link

0xAadi - `deposit()` function in `AssetsProcess` contract fails to restrict a user from depositing amounts greater than `collateralUserCap` #248

Closed sherlock-admin3 closed 1 week ago

sherlock-admin3 commented 2 weeks ago

0xAadi

Medium

deposit() function in AssetsProcess contract fails to restrict a user from depositing amounts greater than collateralUserCap

Summary

The deposit() function in AssetsProcess is intended to handle deposit calls from the AccountFacet contract and update the account's token balance. The function includes checks to ensure the total deposit does not exceed collateralTotalCap and collateralUserCap. While the collateralTotalCap constraint is applied correctly, the code fails to enforce the collateralUserCap. This vulnerability allows a user to deposit an amount greater than collateralUserCap.

Vulnerability Detail

The issue lies in the deposit() function in the AssetsProcess contract.

File: contracts/process/AssetsProcess.sol

91:    CommonData.Props storage commonData = CommonData.load();
92:    uint256 collateralAmount = commonData.getTradeTokenCollateral(token);
93:    if (collateralAmount + params.amount > tradeTokenConfig.collateralTotalCap) {
94:        revert Errors.CollateralTotalCapOverflow(token, tradeTokenConfig.collateralTotalCap);
95:    }
96:@>> if (accountProps.getTokenAmount(token) > tradeTokenConfig.collateralUserCap) {
97:        revert Errors.CollateralUserCapOverflow(token, tradeTokenConfig.collateralUserCap);
98:    }
99:    commonData.addTradeTokenCollateral(token, params.amount);

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/process/AssetsProcess.sol#L91C1-L99C70

As seen in line 96 of the code, the current deposit amount (params.amount) is not considered while checking the collateralUserCap. This allows a user to deposit an amount greater than the collateralUserCap.

Impact

This vulnerability allows users to deposit an amount larger than the collateralUserCap. This will give an unfair advantage to some users who exploit this vulnerability and deposit very large amounts initially.

Code Snippet

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/process/AssetsProcess.sol#L96

Tool used

Manual Review

Recommendation

Update the deposit function to account for the current deposit amount while applying the collateralUserCap constraint.

    if (accountProps.getTokenAmount(token) + params.amount > tradeTokenConfig.collateralUserCap) {
        revert Errors.CollateralUserCapOverflow(token, tradeTokenConfig.collateralUserCap);

Duplicate of #262