Penrose - Deposits protocol fees as WETH, not TAP

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/Penrose.sol#L513-L519

Vulnerability details

Impact

The penrose contract has functionality that enables anyone to call withdrawAllMarketFees function that essentially withdraws all protocol fees and sends them to the 'feeTo' address, that is set by the owner. the natspec in code clearly specifies that these fees should be withdrawn as TAP

    /// @dev Fees are withdrawn in TAP and sent to the FeeDistributor contract

    /// @notice Withdraw the balance of `feeTo`, swap asset into TAP and deposit it to yieldBox of `feeTo`

the swap to TAP should be done within the _depositFeesToYieldBox helper function.

        if (assetId != wethAssetId) {
        yieldBox.transfer(
            address(this),
            address(swapper),
            assetId,
            feeShares
        );
        // @audits swaps fees from asset type to WETH
        ISwapper.SwapData memory swapData = swapper.buildSwapData(
            assetId, // @audit from asset
            wethAssetId, // @audit to asset
            0,
            feeShares,
            true,
            true
        );
        (amount, ) = swapper.swap(
            swapData,
            dexData.minAssetAmount,
            feeTo,
            ""
        );

However upon inspection it seems the function is swapping the contracts base asset to WETH, not TAP, which means the fee distribution contract will receive fees as WETH. this is a clear delineation to the stated requirements by developers.

Proof of Concept

problem is self explanatory

Tools Used

Manual Review

Recommended Mitigation Steps

update code as seen below:

        if (assetId != tapAssetId) { //@audit edit here
        yieldBox.transfer(
            address(this),
            address(swapper),
            assetId,
            feeShares
        );
        ISwapper.SwapData memory swapData = swapper.buildSwapData(
            assetId,
            tapAssetId, //@audit edit here
            0,
            feeShares,
            true,
            true
        );

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as duplicate of #1227

[c4-judge]

dmvt marked the issue as unsatisfactory: Overinflated severity