code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/main/paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol#L229-L265

Vulnerability details

Impact

if the mint was initiated by a contract, then the contract is checked for its ability to receive ERC721 tokens. Without reentrancy guard, onERC721Received will allow an attacker controlled contract to call the mint again, which may not be desirable to some parties, like allowing minting more than allowed. https://www.paradigm.xyz/2021/08/the-dangers-of-surprising-code

executeAcceptBidWithCredit has internal function _acceptBidWithCredit ;

MarketplaceLogic.sol#L247

_acceptBidWithCredit has internal function _repay ;

MarketplaceLogic.sol#L353

_repay has internal function _transferAndCollateralize ;

MarketplaceLogic.sol#L503

_transferAndCollateralize function use IERC721.safeTransferFrom and this function has re-entrancy vulnerability

MarketplaceLogic.sol#L593

 function _transferAndCollateralize(
        mapping(address => DataTypes.ReserveData) storage reservesData,
        DataTypes.UserConfigurationMap storage userConfig,
        MarketplaceLocalVars memory vars,
        address token,
        uint256 tokenId,
        address onBehalfOf
    ) internal {
        uint256[] memory tokenIds = new uint256[](1);
        tokenIds[0] = tokenId;

        IERC721(vars.xTokenAddress).safeTransferFrom(
            address(this),
            onBehalfOf,
            tokenId
        );
        SupplyLogic.executeCollateralizeERC721(
            reservesData,
            userConfig,
            token,
            tokenIds,
            onBehalfOf
        );
    }

Recommended Mitigation Steps

Use Openzeppelin or Solmate Re-Entrancy pattern Here is a example of a re-entrancy guard;

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

contract ReEntrancyGuard {
    bool internal locked;

    modifier noReentrant() {
        require(!locked, "No re-entrancy");
        locked = true;
        _;
        locked = false;
    }
}