code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol#L63-L103 https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol#L575-L591 https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol#L569-L573

Vulnerability details

Impact

When user send extra ETH to executeBuyWithCredit() or executeBatchBuyWithCredit() and tries to buy WETH token, contract would convert user ETH to WETH but it won't transfer back the extra amount user has been sent to contract. (when it doesn't convert the ETH contract sent back the extra amount). so users would lose funds in some conditions, if buying tokens has no exact price then users needs to send extra ETH and those extra amount would be lost.

Proof of Concept

This is executeBuyWithCredit() and _depositETH() code in Marketplace Logic contract:

   function executeBuyWithCredit(
        bytes32 marketplaceId,
        bytes calldata payload,
        DataTypes.Credit calldata credit,
        DataTypes.PoolStorage storage ps,
        IPoolAddressesProvider poolAddressProvider,
        uint16 referralCode
    ) external {
        MarketplaceLocalVars memory vars;

        vars.weth = poolAddressProvider.getWETH();
        DataTypes.Marketplace memory marketplace = poolAddressProvider
            .getMarketplace(marketplaceId);
        DataTypes.OrderInfo memory orderInfo = IMarketplace(marketplace.adapter)
            .getAskOrderInfo(payload, vars.weth);
        orderInfo.taker = msg.sender;
        vars.ethLeft = msg.value;

        _depositETH(vars, orderInfo);

        vars.ethLeft -= _buyWithCredit(
            ps._reserves,
            ps._reservesList,
            ps._usersConfig[orderInfo.taker],
            DataTypes.ExecuteMarketplaceParams({
                marketplaceId: marketplaceId,
                payload: payload,
                credit: credit,
                ethLeft: vars.ethLeft,
                marketplace: marketplace,
                orderInfo: orderInfo,
                weth: vars.weth,
                referralCode: referralCode,
                reservesCount: ps._reservesCount,
                oracle: poolAddressProvider.getPriceOracle(),
                priceOracleSentinel: poolAddressProvider.getPriceOracleSentinel()
            })
        );

        _refundETH(vars.ethLeft);
    }

    function _depositETH(
        MarketplaceLocalVars memory vars,
        DataTypes.OrderInfo memory orderInfo
    ) internal {
        if (
            vars.ethLeft > 0 &&
            orderInfo.consideration[0].itemType != ItemType.NATIVE
        ) {
            IWETH(vars.weth).deposit{value: vars.ethLeft}();
            IERC20(vars.weth).safeTransferFrom(
                address(this),
                msg.sender,
                vars.ethLeft
            );
            vars.ethLeft = 0;
        }
    }

As you can see in _depositETH() when contract convert ETH to WETH it sets ethLeft as zero and function _refundETH(vars.ethLeft) won't return the extra amount to user. the code don't support returning the extra ETH amount when it is converted to wETH. the issue happens when:

user wants to buy an NFT with price is about 100 wETH
user calls executeBuyWithCredit() by sending 110 ETH (user is not sure about exact price and wants order not to fail) and the buying order info which token is wETH.
contract would convert 110 ETH to 110wETH and buy the NFT for user for 100 wETH but it won't send back the extra 10 wETH to user or it won't supply it as collateral for user.

if user buy the token with ETH contract would send back extra amount but for wETH orders users would lose funds.

Tools Used

VIM