VendorLicenseEngine Manual Review Findings

VLE-01M: Inconsistent Input Validation

Type	Severity	Location
Input Sanitization		VendorLicenseEngine.sol:L63, L100-L106

Description:

The value of maxPoolCount is guaranteed to be non-zero when minting a new license, however, when adjusting it a value of zero is permitted.

Impact:

An incorrect sanitization of the maximum pools can lead to other checks that rely on the non-zero trait (such as the existence check we recommended) to misbehave.

Example:

Copy
100function setMaxPoolCount(uint256 _lic, uint256 _maxPoolCount)
101    external
102    onlyOwner
103{
104    if (!licenses[_lic].exists) revert LicenseNotFound();
105    licenses[_lic].maxPoolCount = _maxPoolCount;
106}

Recommendation:

We advise the latter case to be prohibited as a license with no available candidates is illogical. If the desire is to nullify a license altogether, we advise either a dedicated deletion function or the setCurrentPoolCount function to be utilized instead.

Alleviation:

The _maxPoolCount argument is now properly sanitized as non-zero alleviating this exhibit.

VLE-02M: Inexistent Input Validation

Type	Severity	Location
Input Sanitization		VendorLicenseEngine.sol:L108-L114

Description:

The currentPoolCount should always be less-than-or-equal-to (<=) the maxPoolCount, however, no such restriction is imposed on the setter function.

Impact:

A discrepant currentPoolCount in relation to the maxPoolCount can cause logical checks performed by other contracts to fail / succeed incorrectly.

Example:

Copy
108function setCurrentPoolCount(uint256 _lic, uint256 _currentPoolCount)
109    external
110    onlyOwner
111{
112    if (!licenses[_lic].exists) revert LicenseNotFound();
113    licenses[_lic].currentPoolCount = _currentPoolCount;
114}

Recommendation:

We advise this trait to be enforced via a require check or if-revert pattern to ensure the system's state cannot become corrupted.

Alleviation:

The setCurrentPoolCount function now properly validates that the new value set does not exceed the maxPoolCount of the license thereby alleviating this exhibit.

VLE-03M: Inexistent Conformity to Checks Effects Interactions Pattern

Type	Severity	Location
Language Specific		VendorLicenseEngine.sol:L66-L75

Description:

The way licenses are minted causes a corrupted state to be accessible as the license NFT is first minted and then assigned its details and restrictions, permitting the recipient of the license to re-enter other system components with corrupted (i.e. zeroed out) license data points.

Impact:

A corrupted license state can cause misbehaviours across the board, such as underflows in calculating the maximum available pools and other similar caveats.

Example:

Copy
53function safeMint(
54    address to,
55    uint48 _expiry,
56    uint256 _maxPoolCount,
57    uint48 _discount,
58    uint48 _colDiscount
59) public onlyOwner {
60    if (_discount < 0 || _discount > 1000000) revert InvalidDiscount();
61    if (_colDiscount < 0 || _colDiscount > 1000000) revert InvalidDiscount();
62    if (_expiry <= block.timestamp) revert InvalidDiscount();
63    if (_maxPoolCount == 0) revert InvalidDiscount();
64    uint256 tokenId = _tokenIdCounter.current();
65    _tokenIdCounter.increment();
66    _safeMint(to, tokenId);
67    LicenseInfo memory lic = LicenseInfo({
68        maxPoolCount: _maxPoolCount,
69        currentPoolCount: 0,
70        discount: _discount,
71        colDiscount: _colDiscount,
72        expiry: _expiry,
73        exists: true
74    });
75    licenses[tokenId] = lic;
76}

Recommendation:

We advise the system to either invoke _safeMint after assigning the license details to the licenses mapping or the contract to use the _mint instruction instead which prevents re-entrancies altogether.

Alleviation:

The _safeMint statement has been relocated properly after state changes thus conforming to the CEI pattern in full.