Omniscia Vendor Finance Audit

VendorFeesManager Manual Review Findings

VendorFeesManager Manual Review Findings

VFM-01M: Potentially Improper Access Control

Description:

The way the setPoolFees function applies access control permits the callers to arbitrarily set the fees for contracts they control.

Example:

contracts/VendorFeesManager.sol
53function setPoolFees(address _pool, uint48 _feeRate, uint256 _type) external {
54 if (_type < 1 || _type > 2 ) revert InvalidType();
55 if (rateFunction[_pool] != 0 && rateFunction[_pool] != _type) revert InvalidType();
56 if (msg.sender == address(factory) || ILendingPool(_pool).owner() == msg.sender) {
57 feeRates[_pool] = _feeRate;
58 rateFunction[_pool] = _type;
59 emit ChangeFee(_pool, _feeRate, _type);
60 } else {
61 revert NoPermission();
62 }
63}

Recommendation:

We advise this trait to be carefully evaluated and potentially prohibited by imposing an additional level of validation that the _pool is indeed deployed by the pool factory.

Alleviation:

An additional validation level was introduced ensuring the pool is actively deployed by the factory and thus does not represent an arbitrary contract.

VFM-02M: Potentially Unsafe Casting Operation

Description:

The linked casting operation to uint48 is performed unsafely.

Impact:

An improper expiry value can cause a casting overflow to occur truncating the value of the expiration and improperly calculating the current rate.

Example:

contracts/VendorFeesManager.sol
90return (feeRates[_pool] * uint48((ILendingPool(_pool).expiry() - block.timestamp))) / SECONDS_IN_YEAR;

Recommendation:

We advise it to be performed safely by validating that the value being cast does not exist the maximum of a uint48.

Alleviation:

A require check was introduced ensuring that the block.timestamp is within an acceptable range thus guaranteeing that no overflow can occur due to casting truncation. As a result, we consider this exhibit adequately addressed.