Omniscia Nevermined Audit
DIDRegistryLibrary Manual Review Findings
DIDRegistryLibrary Manual Review Findings
DIL-01M: Invalid Royalty Validitation
Type | Severity | Location |
---|---|---|
Mathematical Operations | Major | DIDRegistryLibrary.sol:L159, L179 |
Description:
The areRoyaltiesValid
function is susceptible to overflows in two of the 5 evaluations it conducts.
Example:
contracts/registry/DIDRegistryLibrary.sol
155// If (sum(_amounts) == 0) - It means there is no payment so everything is valid156// returns true;157uint256 _totalAmount = 0;158for(uint i = 0; i < _amounts.length; i++)159 _totalAmount = _totalAmount + _amounts[i];160if (_totalAmount == 0)161 return true;162
163// If (_did.creator is not in _receivers) - It means the original creator is not included as part of the payment164// return false;165bool found = false;166uint256 index;167for (index = 0; index < _receivers.length; index++) {168 if (_self.didRegisters[_did].creator == _receivers[index]) {169 found = true;170 break;171 }172}173
174if (!found) // The creator royalties are not part of the rewards175 return false;176
177// If the amount to receive by the creator is lower than royalties the calculation is not valid178// return false;179uint256 _requiredRoyalties = ((_totalAmount * _self.didRegisters[_did].royalties) / 100);180
181// Check if royalties are enough182// Are we paying enough royalties in the secondary market to the original creator?183return (_amounts[index] >= _requiredRoyalties);
Recommendation:
We advise SafeMath
to be utillized for the summation of the amounts as well as the calculation of the royalty percentage as both can in edge cases overflow.
Alleviation:
The linked statement's raw mathematical operations (addition and multiplication) were replaced by SafeMath
invocations thus preventing an overflow from occuring and properly validating the inputs.