FeeCollector Static Analysis Findings

FCR-01S: Data Location Optimizations

Type	Severity	Location
Gas Optimization		FeeCollector.sol:L136

Description:

The linked input arguments are set as memory in external function(s).

Example:

Copy
136function collectFees(address[] memory _borrowers, address[] memory _assets) external override {

Recommendation:

We advise them to be set as calldata optimizing their read-access gas cost.

Alleviation:

All input arguments of the FeeCollector::collectFees function have been adjusted to calldata, optimizing their read-access gas cost significantly.

FCR-02S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		FeeCollector.sol:L42-L63

Description:

The linked function(s) accept address arguments yet do not properly sanitize them.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

Copy
42function setAddresses(
43    address _borrowerOperationsAddress,
44    address _vesselManagerAddress,
45    address _grvtStakingAddress,
46    address _debtTokenAddress,
47    address _treasuryAddress,
48    bool _routeToGRVTStaking
49) external initializer {
50    require(!isInitialized);
51    require(_treasuryAddress != address(0));
52    borrowerOperationsAddress = _borrowerOperationsAddress;
53    vesselManagerAddress = _vesselManagerAddress;
54    grvtStaking = IGRVTStaking(_grvtStakingAddress);
55    debtTokenAddress = _debtTokenAddress;
56    treasuryAddress = _treasuryAddress;
57    routeToGRVTStaking = _routeToGRVTStaking;
58    if (_routeToGRVTStaking && address(grvtStaking) == address(0)) {
59        revert FeeCollector__InvalidGRVTStakingAddress();
60    }
61    __Ownable_init();
62    isInitialized = true;
63}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that each address specified is non-zero.

Alleviation:

The Gravita Protocol team has opted to not apply a remediation for this exhibit thus rendering it acknowledged.

FCR-03S: Improper Invocations of EIP-20 transfer

Type	Severity	Location
Standard Conformity		FeeCollector.sol:L343, L358

Description:

The linked statement do not properly validate the returned bool of the EIP-20 standard transfer function. As the standard dictates, callers must not assume that false is never returned.

Impact:

If the code mandates that the returned bool is true, this will cause incompatibility with tokens such as USDT / Tether as no such bool is returned to be evaluated causing the check to fail at all times. On the other hand, if the token utilized can return a false value under certain conditions but the code does not validate it, the contract itself can be compromised as having received / sent funds that it never did.

Example:

Copy
343IDebtToken(debtTokenAddress).transfer(collector, _feeAmount);

Recommendation:

Since not all standardized tokens are EIP-20 compliant (such as Tether / USDT), we advise a safe wrapper library to be utilized instead such as SafeERC20 by OpenZeppelin to opportunistically validate the returned bool only if it exists in each instance.

Alleviation:

Both EIP-20 transfer instances now utilize their safe-prefixed counterparts, ensuring that they are performed safely regardless of the underlying EIP-20 implementation.