Wemix Retainer 123 PlayNftMarket

In the contract `PlayMarketOfferBiddingOrder` the role `_editor` has authority over the functions shown in the diagram below. Any compromise to the `_editor` account may allow the hacker to take advantage of this authority and - call `setMinimumPrice()` to set the NFT minimum price. - call `setOrderExpirationPeriod()` to set expiration period of orders. - call `setOfferExpirationPeriod()` to set expiration period of offers.  ----- In the contract `PlayMarketBaseV5` the role `_editor` has authority over the functions shown in the diagram below. Any compromise to the `_editor` account may allow the hacker to take advantage of this authority and - call `addStrategy()` to add or modify strategies to the market. - call `subStrategy()` to inactivate a strategy of the market. - call `setPriceOracle()` to set the `priceOracle` address, which is not used in current protocol. - call `setWhiteList()` to set the whitelist contract. The whitelisted accounts can call paused functions. - call `setTokenManagerAddress()` to change `tokenManagerAddress`, which ensures only whitelisted collections can be traded in the market.  In the contract `PlayMarketBaseV5` the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and - call `pauseFunction()` to pause a function. - call `resumeFunction()` to resume a function. - call `pauseAllFunction()` to pause all functions. - call `resumeAllFunction()` to disable global function pause. 

The `PlayMarketBaseV5` contract inherits upgradeable contracts, indicating that it is part of an upgradeable system. Upgradeable contracts often pair with a proxy contract that is responsible for managing contract upgrades. The `privileged roles` of the proxy often have the authority to update the implementation contract. Any compromise of the privileged account could allow a hacker to exploit this authority, potentially altering the implementation contract pointed to by the proxy and thus executing malicious functionality within the implementation contract.

The `registerSellOrder()` function prevents the registration of the same NFT multiple times by checking if a valid `OFFER_BIDDING` order already exists. It ensures the order ID of the NFT `tokenId` to be sold is not zero: ```solidity=173 if (_registeredOnOfferBidding[_order.collection][_order.tid] != 0) { ``` However, the initial order ID returned by the `OrderIdManager::getOrderId()` function is zero because the `_ordersID` counter is initially set to zero and not incremented before being returned: ```solidity=27 function getOrderId() external nonReentrant isWhitelist(whitelist, msg.sender) returns(uint){ uint orderID = _ordersID.current(); _ordersID.increment(); return orderID; // <= NOT return the increased ID } ``` If the first order is an `OFFER_BIDDING` order, its order ID will be zero. Since the sellers do not need to escrow the NFT for an `OFFER_BIDDING` order, they can register the order again, leading to multiple registrations of the same NFT. As a result, the 1st and the 2nd orders both are selling the same NFT, leaving one of the orders invalid.

The `Order.canceledTime` variable is intended to track the cancellation time of orders. However, this variable is not updated to `block.timestamp` when orders are canceled.

The `resumeAllFunction()` resumes all paused functions by setting the `allPause` state variable to `true`. However, if a specific function has been paused using the `pauseFunction(FUNCTIONS)` function, calling `resumeAllFunction()` will not resume this individually paused function. This behavior can be misleading, as the name `resumeAllFunction()` suggests that all paused functions will be resumed, but in reality, specific functions paused by `pauseFunction(FUNCTIONS)` remain paused.

The `offerBuyOrder()` function allows the caller to specify a different address as the `offerer`. However, when emitting the `OrderAccepted` event, the function incorrectly passes the `offerer` address as the `sender`, instead of `msg.sender`: ```solidity=245 emit OrderAccepted( offerer, // <= should be msg.sender ``` ```solidity=46 event OrderAccepted( address indexed sender, ``` --- The offered price is `_offerPrice[_order.orderID][offerer]` instead of `price`, as users can increase their offer by `price`: ```solidity File: PlayMarketOfferBiddingOrder.sol 71: _offerPrice[_order.orderID][offerer] += price; ... 78: emit OrderOffered(orderID, offerer, price, block.timestamp); ```

The `acceptOffer()` function is used for sellers to accept `OFFER_BIDDING` orders. While the function ensures that the type of orders to be accepted cannot be `BUY`, it does not prevent the acceptance of `SELL` or `AUCTION` orders: ```solidity=318 require(!(_order.orderStrategyType == Type.OrderStrategyType.BUY), "MB: this function is not for buy order"); ```

The `cancelExpiredOrders()` function in the `PlayMarketOpenPriceOrder` contract is designed to cancel orders that have exceeded their duration time. The mapping `durationTime(orderId => durationTime)` stores the duration time for each `orderId`. However, the `cancelExpiredOrders()` function incorrectly accesses this mapping using the loop index `i` instead of the `_order.orderId`: ```solidity File: PlayMarketOpenPriceOrder.sol 14: mapping(uint => uint) public durationTime; // order id => duration time ... 153: function cancelExpiredOrders(uint[] memory orderIDs) external nonReentrant isWhitelist(whitelist, msg.sender) { ... 160: if(durationTime[i] > 0) { // <= array index should be _order.orderId 161: if (_order.startTime + durationTime[i] > block.timestamp) continue; 162: } else{ ``` This error results in the function ignoring `durationTime` configuration and canceling orders based on `stay` duration time.

The `offerSellOrder()` function in the `PlayMarketBaseV5` contract is marked as `payable`, allowing buyers to pay WEMIX coins to purchase NFTs from the market. However, there are inconsistent with the handling of these payments based on the `order.orderStrategyType`. The function calls `OrderV2::offer()` based on the `order.orderStrategyType`: - If `order.orderStrategyType == Type.OrderStrategyType.SELL`, the `SELL` orders are handled by the `PlayMarketFixedPriceOrder::offer()` function, which supports both WEMIX coins and ERC20 tokens: ```solidity=53 if (_order.payment == address(0)) { require(msg.value == orderPrice, "FP : Invalid value"); } else { IERC20(_order.payment).safeTransferFrom(payer, address(this), orderPrice); } ``` - If `order.orderStrategyType == Type.OrderStrategyType.OFFER_BIDDING`, the `OFFER_BIDDING` orders are handled by the `PlayMarketOfferBiddingOrder::offer()` function, which does not support WEMIX coin payments. Additionally, the `offerSellOrder()` function does not forward the received WEMIX coins to the `OrderV2` strategy contract: ```solidity=283 (success, order) = IOrderV2(strategy.strategyAddress).offer(orderID, msg.sender, offerer, amount, marketType); ``` As a result, while the contract intends to support WEMIX payments, the current implementation fails to do so properly. This leads to WEMIX coins being locked in the contract if buyers pay WEMIX to buy NFT.

The `PlayMarketBaseV5` contract allows NFT owners to register `OFFER_BIDDING` orders without escrowing their NFTs or paying any fees. These orders can be settled only when sellers accept the buyer's offers. This creates an opportunity for malicious users to register `OFFER_BIDDING` orders that will never be settled with prices lower than the market value, thereby causing honest users to pay tokens and submit offers on these fraudulent orders. Since these orders will not be settled by malicious sellers, honest users are forced to cancel their offers after realizing they have been deceived, leading to a poor user experience. The only cost to the malicious users is the low WEMIX chain gas fee.