sherlock-audit / 2023-06-symmetrical-judging

5 stars 4 forks source link

SanketKogekar - The function `LibQuote.returnTradingFee` and `LibQuote.receiveTradingFee` misses an important check which can cause loss for the protocol. #330

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

SanketKogekar

medium

The function LibQuote.returnTradingFee and LibQuote.receiveTradingFee misses an important check which can cause loss for the protocol.

Summary

The functions miss the check to make sure the user (or the protocol) has enough funds to cover the tradingFee.

In case the protocol or user doesn't have enough funds to cover the trading fee, the user's action should revert.

Vulnerability Detail

Before increasing the fund balance of protocol and deducting trading fee from allotted balance of user (or deducting the protocol's balance and increasing allotted balance of user), it is necessary to make sure that there are enough funds to cover the trading fee.

In case that there is not enough balance, the function call should revert.

Basically, the functions misses the following check in returnTradingFee and receiveTradingFee

require(accountLayout.balances[GlobalAppStorage.layout().feeCollector] >= tradingFee, "Not enough funds to cover the fee")

Impact

Incorrect balance is updated for protocol & user.

Code Snippet

https://github.com/sherlock-audit/2023-06-symmetrical/blob/6d2b64b6732fcfbd07c8217897dd233dbb6cd1f5/symmio-core/contracts/libraries/LibQuote.sol#L135-L140

https://github.com/sherlock-audit/2023-06-symmetrical/blob/6d2b64b6732fcfbd07c8217897dd233dbb6cd1f5/symmio-core/contracts/libraries/LibQuote.sol#L135-L140

Tool used

Manual Review

Recommendation

Add this require statement to both functions:

require(accountLayout.balances[GlobalAppStorage.layout().feeCollector] >= tradingFee, "incorrect balance")

such that it:

function returnTradingFee(uint256 quoteId) internal {
        //gg
        AccountStorage.Layout storage accountLayout = AccountStorage.layout();
        uint256 tradingFee = LibQuote.getTradingFee(quoteId);
        accountLayout.allocatedBalances[QuoteStorage.layout().quotes[quoteId].partyA] += tradingFee;
        require(accountLayout.balances[GlobalAppStorage.layout().feeCollector] >= tradingFee, "Not enough funds to cover the fee")
        accountLayout.balances[GlobalAppStorage.layout().feeCollector] -= tradingFee;
    }

and do the same for receiveTradingFee as well.