DistributionManager Manual Review Findings

DMR-01M: Incompatibility of Fee-Based Tokens

Type	Severity	Location
Standard Conformity		DistributionManager.sol:L196

Description:

The DistributionManager::receivePayment function will greatly misbehave when dealing with tokens that contain a fee-on-transfer mechanism as it will assume to have received the full amount transferred.

Impact:

The severity of this exhibit will be adjusted depending on whether the Nuklai team intended to support fee-on-transfer tokens in their DistributionManager system.

Example:

Copy
191function receivePayment(address token, uint256 amount) external payable onlySubscriptionManager nonReentrant {
192  if (_versionedTagWeights.length == 0) revert TAG_WEIGHTS_NOT_INITIALIZED();
193  if (address(token) == address(0)) {
194    if (amount != msg.value) revert MSG_VALUE_MISMATCH(msg.value, amount);
195  } else {
196    IERC20(token).safeTransferFrom(_msgSender(), address(this), amount);
197  }
198  uint256 snapshotId = fragmentNFT.snapshot();
199
200  // Deployer fee
201  uint256 deployerFee = (amount * dataset.deployerFeePercentage(datasetId)) / 1e18;

Recommendation:

We advise the code to either explicitly specify that fee-on-transfer tokens are not supported or to properly support them by tracking the before-and-after balance measurements of the contract, either of which we consider an adequate resolution to this exhibit.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The documentation of the DistributionManager::receivePayment function was updated to clearly specify that fee-on-transfer tokens are not supported.

We would like to note that the Nuklai team specified the main use-case of the contract being stablecoins; most stablecoins contain the possibility to implement a fee-on-transfer, however, it is very unlikely that this feature will be activated in the near future.

Given that the documentation was updated to correctly illustrate the absence of support for fee-on-transfer tokens, we consider this exhibit sufficiently dealt with.

DMR-02M: Incorrect Initialization Methodology

Type	Severity	Location
Logical Fault		DistributionManager.sol:L91

Description:

The DistributionManager::initialize function will incorrectly initialize the distribution manager when no fragment NFT has been deployed yet for a particular datasetId as the DatasetNFT::setManagers function permits the configuration of a distributionManager even if no fragment NFT has yet been deployed.

Impact:

A misconfiguration of the DistributionManager would be undetected by front-end processes as the DatasetNFT::setManagers function can be invoked at any time, leading to the vulnerability manifesting when the DistributionManager attempts to receive funds via the DistributionManager::receivePayment function.

Example:

Copy
87function initialize(address dataset_, uint256 datasetId_) external initializer {
88  __ReentrancyGuard_init();
89  dataset = IDatasetNFT(dataset_);
90  datasetId = datasetId_;
91  fragmentNFT = IFragmentNFT(dataset.fragmentNFT(datasetId));
92}

Recommendation:

We advise the code to properly ensure that the fragmentNFT entry extracted from the dataset is non-zero, reverting in any other case to prevent deploying a DistributionManager with a misconfigured fragmentNFT.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The DatasetNFT::setManagers function was instead updated, ensuring that a fragment has been deployed for the relevant id thus guaranteeing that the DatasetNFT::fragmentNFT getter function will yield a non-zero result.

As such, we consider this exhibit alleviated at the DatasetNFT contract.

DMR-03M: Potential Loss of Funds

Type	Severity	Location
Language Specific		DistributionManager.sol:L196

Description:

The DistributionManager::receivePayment function does not prevent native funds from being sent alongside EIP-20 assets, leading to a potential loss of funds that are transmitted in such a call.

Impact:

It is presently possible to lock native funds in the DistributionManager contract by specifying a non-zero amount of native funds alongside a DistributionManager::receivePayment call with an EIP-20 asset.

Example:

Copy
193if (address(token) == address(0)) {
194  if (amount != msg.value) revert MSG_VALUE_MISMATCH(msg.value, amount);
195} else {
196  IERC20(token).safeTransferFrom(_msgSender(), address(this), amount);
197}

Recommendation:

We advise the code to ensure that the msg.value is 0 in case the token specified is non-zero, ensuring that native funds cannot be accidentally locked in the contract when supplying native funds alongside an EIP-20 DistributionManager::receivePayment function execution.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The DistributionManager::receivePayment code was properly updated to ensure that the msg.value is non-zero in the else execution flow accepting EIP-20 assets, alleviating this exhibit in full.