sherlock-audit / 2023-06-symmetrical-judging

5 stars 4 forks source link

simon135 - FeeCollector can get ouf WithdrawCooldown in `receiveTradingFees` #318

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

simon135

medium

FeeCollector can get ouf WithdrawCooldown in receiveTradingFees

Summary

since receiveTradingFees function uses the balance it doesn't need to wait the cooldown

Vulnerability Detail

  1. fee collector can skip the deallocating process because in the code it uses balance instead of allocated PartyAs' allocate 1000 units of tokens to balance Of the fee collector and then they can just withdraw so its a way for suspended users to clear their balance and get out of that state
  2. fee collector can cause most functions in the protocol to revert by subtracting their balance and making that call revert( it can also be not malicious because if the protocol wants some of the profits during the positions some of the calls will revert)

    Impact

    FeeCollector being the attacker and getting out of suspended and it goes against the spec that users need to wait to withdraw allocated

    Code Snippet

        AccountStorage.Layout storage accountLayout = AccountStorage.layout();
        uint256 tradingFee = LibQuote.getTradingFee(quoteId);
        accountLayout.allocatedBalances[QuoteStorage.layout().quotes[quoteId].partyA] -= tradingFee;
        accountLayout.balances[GlobalAppStorage.layout().feeCollector] += tradingFee;
    }

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

    Tool used

    Forge Manual Review

Recommendation

Dont use Balance instead use allocated like PartyA

Duplicate of #299

snn20 commented 1 year ago

escalate

299 states how FeeCollector can change the balance and make the functions revert.

This issue states how FeeCollecotor can get out the withdrawCooldown The recommendation is similar but the impact/invariant in this issue is different.

sherlock-admin2 commented 1 year ago

escalate

299 states how FeeCollector can change the balance and make the functions revert.

This issue states how FeeCollecotor can get out the withdrawCooldown The recommendation is similar but the impact/invariant in this issue is different.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

JeffCX commented 1 year ago

https://github.com/sherlock-audit/2023-06-symmetrical-judging/issues/299#issuecomment-1653583620

hrishibhat commented 1 year ago

Result: Medium Duplicate of #299 This is a valid duplicate of #299 as the underlying issue is the same.

sherlock-admin2 commented 1 year ago

Escalations have been resolved successfully!

Escalation status: