Omniscia Nevermined Audit

DIDRegistry Manual Review Findings

DIDRegistry Manual Review Findings

DIR-01M: Provenance Out of Sync

TypeSeverityLocation
Logical FaultMediumDIDRegistry.sol:L119-L140

Description:

The super.used call that registers the provenance will not be reflected in the blockchain during the EIP-1155 callback of _mint thereby misbehaving.

Example:

contracts/registry/DIDRegistry.sol
119function mint(
120 bytes32 _did,
121 uint256 _amount
122)
123public
124onlyDIDOwner(_did)
125nftIsInitialized(_did)
126{
127 if (didRegisterList.didRegisters[_did].mintCap > 0) {
128 require(
129 didRegisterList.didRegisters[_did].nftSupply + _amount <= didRegisterList.didRegisters[_did].mintCap,
130 'Cap exceeded'
131 );
132 }
133
134 didRegisterList.didRegisters[_did].nftSupply = didRegisterList.didRegisters[_did].nftSupply.add(_amount);
135 super._mint(msg.sender, uint256(_did), _amount, '');
136
137 super.used(
138 keccak256(abi.encodePacked(_did, msg.sender, 'mint', _amount, block.number)),
139 _did, msg.sender, keccak256('mint'), '', 'mint');
140}

Recommendation:

We advise the used call to be performed prior to the minting of the NFT to ensure that the Checks-Effects-Interactions pattern is conformed to.

Alleviation:

The statements were properly reordered to first perform the used call before minting the NFT.

DIR-02M: Ineffectual Cap Validation

Description:

The minting cap validation performs an unsafe addition that can render the check useless.

Example:

contracts/registry/DIDRegistry.sol
119function mint(
120 bytes32 _did,
121 uint256 _amount
122)
123public
124onlyDIDOwner(_did)
125nftIsInitialized(_did)
126{
127 if (didRegisterList.didRegisters[_did].mintCap > 0) {
128 require(
129 didRegisterList.didRegisters[_did].nftSupply + _amount <= didRegisterList.didRegisters[_did].mintCap,
130 'Cap exceeded'
131 );
132 }
133
134 didRegisterList.didRegisters[_did].nftSupply = didRegisterList.didRegisters[_did].nftSupply.add(_amount);
135 super._mint(msg.sender, uint256(_did), _amount, '');
136
137 super.used(
138 keccak256(abi.encodePacked(_did, msg.sender, 'mint', _amount, block.number)),
139 _did, msg.sender, keccak256('mint'), '', 'mint');
140}

Recommendation:

Although the check can be bypassed, the SafeMath addition of the nftSupply will cause the function to revert for an unrelated reason. We recommend a SafeMath addition to be utilized in the require check instead and be omitted from the nftSupply assignment if the gas optimization is desirable.

Alleviation:

The SafeMath addition was included in the require check to ensure that an overflow does not bypass the require check. We still recommend the SafeMath addition at L134 to now be omitted as it has already been safely performed in the require check and thus is not needed.