sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

0x73696d616f - Stuck fees in `MarketFactory` as there is no function to withdraw them #173

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

0x73696d616f

high

Stuck fees in MarketFactory as there is no function to withdraw them

Summary

The MarketFactory claims fees from the Market in function fund(), which are stored in the MarketFactory contract. However, the fees have no way to be retrieved from the contract, leading to stuck funds.

Vulnerability Detail

The Market attributes fees to the MarketFactory via claimFee(). It pushes the fees to the MarketFactory when the function is called by it (via fund()).

Notice that function fund() does not send the received funds anywhere, nor is there another function to retrieve these funds, leading to stuck tokens.

Impact

Stuck funds from fees in the MarketFactory contract.

Code Snippet

Function fund() pulls the funds to the MarketFactory, but does not move them afterwards:

    function fund(IMarket market) external {
        if (!instances(IInstance(address(market)))) revert FactoryNotInstanceError();
        market.claimFee();
    }

Function claimFee() pushes the funds to the respective receiver, which is the MarketFactory in this case:

function claimFee() external {
    Global memory newGlobal = _global.read();

    if (_claimFee(address(factory()), newGlobal.protocolFee)) newGlobal.protocolFee = UFixed6Lib.ZERO;
    if (_claimFee(address(IMarketFactory(address(factory())).oracleFactory()), newGlobal.oracleFee))
        newGlobal.oracleFee = UFixed6Lib.ZERO;
    if (_claimFee(coordinator, newGlobal.riskFee)) newGlobal.riskFee = UFixed6Lib.ZERO;
    if (_claimFee(beneficiary, newGlobal.donation)) newGlobal.donation = UFixed6Lib.ZERO;

    _global.store(newGlobal);
}

/// @notice Helper function to handle a singular fee claim
/// @param receiver The address to receive the fee
/// @param fee The amount of the fee to claim
function _claimFee(address receiver, UFixed6 fee) private returns (bool) {
    if (msg.sender != receiver) return false;

    token.push(receiver, UFixed18Lib.from(fee));
    emit FeeClaimed(receiver, fee);
    return true;
}

Tool used

Vscode, Hardhat, Manual Review

Recommendation

Push the funds to the owner in the fund() function (syntax might be wrong):

function fund(IMarket market) external { // TO SUBMIT cant withdraw
    if (!instances(IInstance(address(market)))) revert FactoryNotInstanceError();
    market.claimFee();
    incentive.push(owner, incentive.balanceOf());
}

Duplicate of #52

sherlock-admin commented 1 year ago

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

141345 commented:

d