VendorFeesManager Manual Review Findings

VFM-01M: Potentially Improper Access Control

Type	Severity	Location
Logical Fault		VendorFeesManager.sol:L53-L63

Description:

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

Example:

Copy
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

Type	Severity	Location
Mathematical Operations		VendorFeesManager.sol:L90

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:

Copy
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.