Omniscia Arcade XYZ Audit
PunksVerifier Manual Review Findings
PunksVerifier Manual Review Findings
PVR-01M: Potentially Unwarranted Predicate Case
| Type | Severity | Location |
|---|---|---|
| Code Style |  | PunksVerifier.sol:L74 |
Description:
The PunksVerifier::verifyPredicates function contains a special verification mechanism whereby a negative tokenId value signifies a simple balance evaluation of the vault.
Example:
58function verifyPredicates(59 address, address,60 address collateralAddress,61 uint256 collateralId,62 bytes calldata predicates63) external view override returns (bool) {64 address vault = IVaultFactory(collateralAddress).instanceAt(collateralId);65
66 // Unpack items67 int256[] memory tokenIds = abi.decode(predicates, (int256[]));68
69 for (uint256 i = 0; i < tokenIds.length; i++) {70 int256 tokenId = tokenIds[i];71
72 if (tokenId > 9999) revert IV_InvalidTokenId(tokenId);73
74 if (tokenId < 0 && punks.balanceOf(vault) == 0) return false;75 // Does not own specifically specified asset76 else if (tokenId >= 0 && punks.punkIndexToAddress(tokenId.toUint256()) != vault) return false;77 }78
79 // Loop completed - all items found80 return true;81}Recommendation:
Given that the CollectionWideOfferVerifier::verifyPredicates function is compatible with the IPunks contract, we advise the functionality within PunksVerifier::verifyPredicates to be omitted given that it can be confusing and is relatively unrestricted (i.e. any negative tokenId value will cause it to execute).
Alleviation (7a4e1dc948e94ded7385dbb74818bcf93ecc207c):
The Arcade XYZ team evaluated this exhibit and stated that they do not plan to deploy the PunksVerifier in the immediate future nor whitelist its operation, meaning that they may potentially revamp the contract.
As such, we consider this exhibit safely acknowledged.
PVR-02M: Incorrect Assumption of Function
| Type | Severity | Location |
|---|---|---|
| Logical Fault |  | PunksVerifier.sol:L48-L49 |
Description:
The PunksVerifier::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 PunksVerifier::verifyPredicates will misbehave if supplied an empty tokenIds data entry when called via the OriginationController despite its documentation specifying that no sanitization is needed as it is incorrect.
Example:
44/**45 * @notice Verify that the items specified by the packed int256 array are held by the vault.46 * @dev Reverts on out of bounds token Ids, returns false on missing contents.47 *48 * Verification for empty predicates array has been addressed in initializeLoanWithItems and49 * rolloverLoanWithItems.50 *51 * @param collateralAddress The address of the loan's collateral.52 * @param collateralId The tokenId of the loan's collateral.53 * @param predicates The int256[] array of punk IDs to check for, packed in bytes.54 *55 * @return verified Whether the bundle contains the specified items.56 */57// solhint-disable-next-line code-complexity58function verifyPredicates(59 address, address,60 address collateralAddress,61 uint256 collateralId,62 bytes calldata predicates63) external view override returns (bool) {64 address vault = IVaultFactory(collateralAddress).instanceAt(collateralId);65
66 // Unpack items67 int256[] memory tokenIds = abi.decode(predicates, (int256[]));68
69 for (uint256 i = 0; i < tokenIds.length; i++) {Recommendation:
We advise the code to properly ensure that the tokenIds 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 int256 array contains non-zero entries, alleviating this exhibit in full.