Omniscia Nexera Protocol Audit

DatasetNFT Manual Review Findings

DatasetNFT Manual Review Findings

DNF-01M: Improper Enforcement Flow of Deployer Fees

Description:

The DatasetNFT::setDeployerFeeModelPercentages function can be invoked even when the contract has no deployerFeeBeneficiary specified. This can cause significant misbehaviours in the DistributionManager::receivePayment function as it will fatally fail if the deployerFeeBeneficiary has not been specified when a non-zero fee is expected to be applied.

Impact:

While the misconfiguration is trivial to resolve, it should be prohibited at the code level as otherwise valid subscriptions may fail to execute due to the DistributionManager::receivePayment function's failure, in turn leading to the subscription being lost as the subscriber may not wish to retry their operation.

Example:

contracts/DatasetNFT.sol
180/**
181 * @notice Sets the fee percentages for the provided Deployer Fee Models
182 * @dev Only callable by DatasetNFT ADMIN.
183 * Percentages are encoded such that 100% is represented as 1e18.
184 * @param models An array of Deployer Fee Models to set percentages for (see `IDatasetNFT.sol`)
185 * @param percentages An array of corresponding fee percentages
186 */
187function setDeployerFeeModelPercentages(
188 DeployerFeeModel[] calldata models,
189 uint256[] calldata percentages
190) external onlyRole(DEFAULT_ADMIN_ROLE) {
191 if (models.length != percentages.length) revert ARRAY_LENGTH_MISMATCH();
192 for (uint256 i; i < models.length; i++) {
193 DeployerFeeModel m = models[i];
194 if (uint8(m) == 0) revert INVALID_ZERO_MODEL_FEE();
195 uint256 p = percentages[i];
196 if (p > 1e18) revert PERCENTAGE_VALUE_INVALID(1e18, p);
197 deployerFeeModelPercentage[m] = p;
198 }
199}
200
201/**
202 * @notice Sets the deployer fee model for a specific Dataset
203 * @dev Only callable by DatasetNFT ADMIN
204 * @param datasetId The ID of the target Dataset NFT token
205 * @param model The Deployer Fee Model to set
206 */
207function setDeployerFeeModel(uint256 datasetId, DeployerFeeModel model) external onlyRole(DEFAULT_ADMIN_ROLE) {
208 deployerFeeModels[datasetId] = model;
209}
210
211/**
212 * @notice Sets the address of the deployer fee beneficiary
213 * @dev Only callable by DatasetNFT ADMIN
214 * @param deployerFeeBeneficiary_ The address to set as the beneficiary
215 */
216function setDeployerFeeBeneficiary(address deployerFeeBeneficiary_) external onlyRole(DEFAULT_ADMIN_ROLE) {
217 if (deployerFeeBeneficiary_ == address(0)) revert ZERO_ADDRESS();
218 deployerFeeBeneficiary = deployerFeeBeneficiary_;
219}

Recommendation:

We advise the DatasetNFT::setDeployerFeeModelPercentages function to ensure that the deployerFeeBeneficiary has been specified to a non-zero value when it is invoked, preventing a non-zero fee from being applied to a particular model and data-set ID when no beneficiary for it has been specified.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

An if-revert pattern check was properly introduced ensuring that the deployerFeeBeneficiary has been specified when the DatasetNFT::setDeployerFeeModelPercentages function is invoked thus alleviating this exhibit in full.

DNF-02M: Inexistent Sanitization of Sensitive Module Implementations

Description:

The DatasetNFT::setManagers function permits the subscriptionManager, distributionManager, and verifierManager implementations that the system will utilize to be arbitrarily specified which is a significant security flaw in the system given that these modules are expected to utilize user funds, get approved by users to spend funds, and generally act as integral modules to the DatasetNFT contract's overall operation.

Impact:

Arbitrarily specified subscription and distribution implementations can lead to direct fund loss (by exploiting approvals) or to "free" data-set contributions (as the distributor could f.e. distribute all funds to a single party).

Example:

contracts/DatasetNFT.sol
160function setManagers(uint256 id, ManagersConfig calldata config) external onlyTokenOwner(id) {
161 bool changed;
162 if (configurations[id].subscriptionManager != config.subscriptionManager) {
163 proxies[id].subscriptionManager = _cloneAndInitialize(config.subscriptionManager, id);
164 changed = true;
165 }
166 if (configurations[id].distributionManager != config.distributionManager) {
167 proxies[id].distributionManager = _cloneAndInitialize(config.distributionManager, id);
168 changed = true;
169 }
170 if (configurations[id].verifierManager != config.verifierManager) {
171 proxies[id].verifierManager = _cloneAndInitialize(config.verifierManager, id);
172 changed = true;
173 }
174 if (changed) {
175 configurations[id] = config;
176 emit ManagersConfigChange(id);
177 }
178}

Recommendation:

We advise the code to maintain an administrator-controlled implementation whitelist per manager type that the DatasetNFT::setManagers function sanitizes its inputs against, ensuring that the implementations attached to a particular data-set ID are secure and valid.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

A DatasetNFT::_checkManager function was introduced to the codebase that ensures the relevant implementations are non-zero, conform to a specific interface type, and have ultimately been whitelisted with the WHITELISTED_MANAGER_ROLE assigned via the AccessControlUpgradeable contract.

As a result of these changes, we consider this exhibit fully alleviated.

DNF-03M: Insecure Signature Validation

Description:

The DatasetNFT::_mintMessageHash function will yield a message digest that does not contain the to member the mint operation will create an NFT to.

As such, all mint operations are susceptible to on-chain race conditions whereby a DatasetNFT::mint operation is detected and a malicious actor submits a transaction with a higher priority that changes the to argument to their choosing.

Impact:

The signature validation mechanism in DatasetNFT::mint is ineffectual as it does not validate the to member the NFT is being minted for, permitting anyone to claim any signature.

Example:

contracts/DatasetNFT.sol
140function mint(bytes32 uuidHashed, address to, bytes calldata signature) external returns (uint256) {
141 bytes32 msgHash = _mintMessageHash(uuidHashed);
142 address signer = ECDSA.recover(msgHash, signature);
143 if (!hasRole(SIGNER_ROLE, signer)) revert BAD_SIGNATURE(msgHash, signer);
144
145 uint256 id = uint256(uuidHashed);
146
147 _mint(to, id);
148
149 return id;
150}

Recommendation:

We advise the code to properly attach the to member with the uuidHashed, preventing on-chain race conditions from being able to "steal" the mint operation of a particular data-set ID.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The DatasetNFT::_mintMessageHash function was updated to properly include the to argument as part of the payload and the overall mint flow was additionally updated to be executed solely via the DatasetFactory contract and specifically its DatasetFactory::mintAndConfigureDataset function.

As a result of these changes, we consider the overall signature validation mechanism secure and the minting function of the DatasetNFT no longer prone to race conditions.