Omniscia Arcade XYZ Audit

ArtBlocksVerifier Manual Review Findings

ArtBlocksVerifier Manual Review Findings

ABV-01M: Improper Enforcement of Check

Description:

The referenced statement is meant to validate that the predicate's amount value is non-zero, however, the predicate's amount is solely utilized when anyIdAllowed is set to true.

Impact:

The code will not assume an amount of 1 if tokenId has been defined in the current iteration of the codebase for validation purposes, rendering it contradictory to its specification.

Example:

contracts/verifiers/ArtBlocksVerifier.sol
95// No amount provided
96if (item.amount == 0) revert IV_NoAmount(item.asset, item.amount);
97

98if (item.anyIdAllowed) {
99    // Iterate through tokens
100    uint256 tokenCount = IArtBlocks(item.asset).balanceOf(vault);
101    uint256 found;
102

103    for (uint256 j = 0; j < tokenCount; j++) {
104        uint256 fullTokenId = IArtBlocks(item.asset).tokenOfOwnerByIndex(vault, j);
105        uint256 ownedProjectId = fullTokenId / PROJECT_ID_BASE;
106

107        // If project is owned, increment num found
108        // If we've found enough, break
109        if (ownedProjectId == item.projectId) {
110            found++;
111

112            if (found >= item.amount) break;
113        }
114    }
115

116    // We looped and didn't find enough, so fail
117    if (found < item.amount) return false;
118} else {
119    // Look for a specific token ID
120    uint256 fullTokenId = _getFullTokenId(item.projectId, item.tokenId);
121

122    // Check if the token is owned by the vault
123    if (IERC721(item.asset).ownerOf(fullTokenId) != vault) {
124        return false;
125    }
126}

Recommendation:

Given that the struct documentation explicitly states amount is assumed to be 1 if a tokenId has been specified, we advise the code to relocate the referenced check within the if (item.anyIdAllowed) code branch, ensuring that it is validated only when it is appropriate to do so.

Alleviation (7a4e1dc948e94ded7385dbb74818bcf93ecc207c):

The referenced if-revert check has been relocated as advised, ensuring that the code behaves according to its specification.

ABV-02M: Incorrect Assumption of Function

Description:

The ArtBlocksVerifier::verifyPredicates function assumes that an empty predicates array has been addressed in the OriginationController::initializeLoanWithItems and OriginationController::rolloverLoanWithItems functions, however, this is incorrect as the functions ensure that the whole predicate array is not empty, not that the data payload of a predicate call contains non-zero entries.

Impact:

The ArtBlocksVerifier::verifyPredicates will misbehave if supplied an empty items data entry when called via the OriginationController despite its documentation specifying that no sanitization is needed as it is incorrect.

Example:

contracts/verifiers/ArtBlocksVerifier.sol
62/**
63 * @notice Verify that the items specified by the packed SignatureItem array are held by the vault.
64 * @dev    Reverts on a malformed SignatureItem, returns false on missing contents.
65 *
66 *         Verification for empty predicates array has been addressed in initializeLoanWithItems and
67 *         rolloverLoanWithItems.
68 *
69 * @param collateralAddress             The address of the loan's collateral.
70 * @param collateralId                  The tokenId of the loan's collateral.
71 * @param predicates                    The SignatureItem[] array of items, packed in bytes.
72 *
73 * @return verified                     Whether the bundle contains the specified items.
74 */
75// solhint-disable-next-line code-complexity
76function verifyPredicates(
77    address, address,
78    address collateralAddress,
79    uint256 collateralId,
80    bytes calldata predicates
81) external view override returns (bool) {
82    address vault = IVaultFactory(collateralAddress).instanceAt(collateralId);
83    // Unpack items
84    SignatureItem[] memory items = abi.decode(predicates, (SignatureItem[]));
85

86    for (uint256 i = 0; i < items.length; ++i) {

Recommendation:

We advise the code to properly ensure that the items decoded contain a non-zero length as otherwise the predicate would succeed without validating anything, signifying a potential scam attempt.

Alleviation (7a4e1dc948e94ded7385dbb74818bcf93ecc207c):

A proper if-revert pattern was introduced that ensures the decoded SignatureItem array contains non-zero entries, alleviating this exhibit in full.

Prev

ArcadeItemsVerifier.sol (AIV-M)

Next

AssetVault.sol (AVT-M)

On this page