Omniscia Seen Haus Audit
SaleBuilderFacet Manual Review Findings
SaleBuilderFacet Manual Review Findings
SBF-01M: Inexistent Sanitization of Sale Start
Type | Severity | Location |
---|---|---|
Input Sanitization | Medium | SaleBuilderFacet.sol:L71, L113, L135, L227 |
Description:
The documentation states that the function should revert
if the _start
of the sale is in the past, however, no such logical guarantee is enforced in the code.
Example:
70* Reverts if:71 * - Sale starts in the past72 * - Sale exists for consignment73 * - Consignment has already been marketed74 *75 * Emits a SalePending event.76 *77 * @param _consignmentId - id of the consignment being sold78 * @param _start - the scheduled start time of the sale79 * @param _price - the price of each item in the lot80 * @param _perTxCap - the maximum amount that can be bought in a single transaction81 * @param _audience - the initial audience that can participate. See {SeenTypes.Audience}82 */83function createPrimarySale (84 uint256 _consignmentId,85 uint256 _start,86 uint256 _price,87 uint256 _perTxCap,88 Audience _audience89)90external91override92onlyRole(SELLER)93onlyConsignor(_consignmentId)94{95 // Get Market Handler Storage slot96 MarketHandlerLib.MarketHandlerStorage storage mhs = MarketHandlerLib.marketHandlerStorage();97
98 // Fetch the consignment99 Consignment memory consignment = getMarketController().getConsignment(_consignmentId);100
101 // Make sure the consignment hasn't been marketed102 require(consignment.marketHandler == MarketHandler.Unhandled, "Consignment has already been marketed");103
104 // Get the storage location for the sale105 Sale storage sale = mhs.sales[consignment.id];106
107 // Make sure sale doesn't exist (start would always be non-zero on an actual sale)108 require(sale.start == 0, "Sale exists");109
110 // Set up the sale111 setAudience(_consignmentId, _audience);112 sale.consignmentId = _consignmentId;113 sale.start = _start;
Recommendation:
We advise such a require
statement to be introduced to the codebase. If the _start
is accidentally set to 0
in the current case, the NFT will be permanently locked.
Alleviation:
The comment has been updated to ensure that a positive value for _start
needs to be passed in and the code has been updated to reflect that via a require
check.
SBF-02M: Inexistent Validation of Proper Supply
Type | Severity | Location |
---|---|---|
Input Sanitization | Minor | SaleBuilderFacet.sol:L209-L214, L219 |
Description:
The ERC-721 workflow for creating a sale does not properly validate that the user-supplied _supply
value is equal to 1
and can cause the system to misbehave.
Example:
182// Make sure supply is non-zero183require (_supply > 0, "Supply must be non-zero");184
185// Make sure this contract is approved to transfer the token186// N.B. The following will work because isApprovedForAll has the same signature on both IERC721 and IERC1155187require(IERC1155Upgradeable(_tokenAddress).isApprovedForAll(msg.sender, address(this)), "Not approved to transfer seller's tokens");188
189// To register the consignment, tokens must first be in MarketController's possession190if (IERC165Upgradeable(_tokenAddress).supportsInterface(type(IERC1155Upgradeable).interfaceId)) {191
192 // Ensure seller owns sufficient supply of token193 require(IERC1155Upgradeable(_tokenAddress).balanceOf(msg.sender, _tokenId) >= _supply, "Seller has insufficient balance of token");194
195 // Transfer supply to MarketController196 IERC1155Upgradeable(_tokenAddress).safeTransferFrom(197 msg.sender,198 address(getMarketController()),199 _tokenId,200 _supply,201 new bytes(0x0)202 );203
204} else {205
206 // Token must be a single token NFT207 require(IERC165Upgradeable(_tokenAddress).supportsInterface(type(IERC721Upgradeable).interfaceId), "Invalid token type");208
209 // Transfer tokenId to MarketController210 IERC721Upgradeable(_tokenAddress).safeTransferFrom(211 msg.sender,212 address(getMarketController()),213 _tokenId214 );215
216}217
218// Register consignment219Consignment memory consignment = getMarketController().registerConsignment(Market.Secondary, msg.sender, _seller, _tokenAddress, _tokenId, _supply);
Recommendation:
We advise such sanitization to be introduced in the else
code block of the ERC-721 transfer.
Alleviation:
A require
check has been introduced ensuring that the _supply
is equal to 1
.