sherlock-audit / 2024-06-symmetrical-update-2-judging

0 stars 0 forks source link

slowfi - `sendQuote` function may always revert #21

Closed sherlock-admin4 closed 1 week ago

sherlock-admin4 commented 2 weeks ago

slowfi

Medium

sendQuote function may always revert

Summary

The new sendQuote function will always revert as PartyAFacetImpl.sol checks on PartyAFacetImpl.sol#L72 that is a valid affiliate. There is not task nor script that registers address(0) as a valid affiliate. Also creating a valid affiliate with address zero may lead to improper storage of collected fees.

Vulnerability Detail

The sendQuoteWithAffiliate function is designed to handle fee distribution by specifying an affiliate address. However, the previous function sendQuote forces the affiliate address to be zero. Due to the check at L72 on PartyAFacetImpl.sol, the function reverts if the affiliate is not valid. Since address(0) cannot be a valid affiliate, this scenario causes the function to fail. Moreover, attempting to create a valid affiliate with address zero would result in improper fee storage, leading to fees being stores on the storage of the zero address.

Impact

Lack of intended operation functionality and/or improper fee handling.

Code Snippet

sendQuote calls `` with address(0) as affiliate. PartyAFacet.sol#L132

    function sendQuote(
        address[] memory partyBsWhiteList,
        uint256 symbolId,
        PositionType positionType,
        OrderType orderType,
        uint256 price,
        uint256 quantity,
        uint256 cva,
        uint256 lf,
        uint256 partyAmm,
        uint256 partyBmm,
        uint256 maxFundingRate,
        uint256 deadline,
        SingleUpnlAndPriceSig memory upnlSig
    ) external whenNotPartyAActionsPaused notLiquidatedPartyA(msg.sender) notSuspended(msg.sender) {
        uint256 quoteId = PartyAFacetImpl.sendQuote(
            partyBsWhiteList,
            symbolId,
            positionType,
            orderType,
            price,
            quantity,
            cva,
            lf,
            partyAmm,
            partyBmm,
            maxFundingRate,
            deadline,
            address(0),
            upnlSig
        );

sendQuote of implementation facet that may revert PartyAFacetImpl.sol#L72

        require(maLayout.affiliateStatus[affiliate], "PartyAFacet: Invalid affiliate");

Tool used

Manual Review

Recommendation

Consider removing this function. Or creating other workflow for address zero affiliates.

Duplicate of #22

sherlock-admin2 commented 1 week ago

1 comment(s) were left on this issue during the judging contest.

Hash01011122 commented:

Low/Info Vulnerability

sherlock-admin2 commented 1 week ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/SYMM-IO/protocol-core/pull/55

sherlock-admin2 commented 3 days ago

The Lead Senior Watson signed off on the fix.