ERC20SubscriptionManager Manual Review Findings

ERC-01M: Inexistent Prevention of Native Funds

Type	Severity	Location
Language Specific		ERC20SubscriptionManager.sol:L91-L96

Description:

The ERC20SubscriptionManager::_charge function will not prevent native funds from being sent alongside the call the function is executed in, resulting in the GenericSingleDatasetSubscriptionManager::subscribe and GenericSingleDatasetSubscriptionManager::extendSubscription functions locking all native funds that may be accidentally sent when they are invoked.

Impact:

It is presently possible to lock funds in the ERC20SubscriptionManager by supplying native funds in either the GenericSingleDatasetSubscriptionManager::subscribe or GenericSingleDatasetSubscriptionManager::extendSubscription functions.

Example:

Copy
91function _charge(address subscriber, uint256 amount) internal override {
92  token.safeTransferFrom(subscriber, address(this), amount);
93  address distributionManager = dataset.distributionManager(datasetId);
94  token.approve(distributionManager, amount);
95  IDistributionManager(distributionManager).receivePayment(address(token), amount);
96}

Recommendation:

We advise the code to introduce a require check, ensuring that the msg.value of the call is 0 to prevent native funds from being accidentally locked in the ERC20SubscriptionManager.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The ERC20SubscriptionManager::_charge function was correctly updated to prevent a non-zero msg.value from being passed in the call it is invoked in, ensuring no funds can be locked as part of the GenericSingleDatasetSubscriptionManager execution flows.