DynamicFeeHook Static Analysis Findings

DFH-01S: Illegible Numeric Value Representations

Type	Severity	Location
Code Style		DynamicFeeHook.sol: • I-1: L47 • I-2: L353

Description:

The linked representations of numeric literals are sub-optimally represented decreasing the legibility of the codebase.

Example:

Copy
47uint256 private constant FEE_DENOMINATOR = 10000; // 100% = 10000 bps

Recommendation:

To properly illustrate each value's purpose, we advise the following guidelines to be followed. For values meant to depict fractions with a base of 1e18, we advise fractions to be utilized directly (i.e. 1e17 becomes 0.1e18) as they are supported. For values meant to represent a percentage base, we advise each value to utilize the underscore (_) separator to discern the percentage decimal (i.e. 10000 becomes 100_00, 300 becomes 3_00 and so on). Finally, for large numeric values we simply advise the underscore character to be utilized again to represent them (i.e. 1000000 becomes 1_000_000).

Alleviation (5dd8944ae71b382923f6971e9985eb7f427147ec):

The referenced value literal has been updated in its representation to 100_00 in accordance with the recommendation's underscore style, addressing this exhibit.

DFH-02S: Inexistent Sanitization of Input Address

Type	Severity	Location
Input Sanitization		DynamicFeeHook.sol:L65-L76

Description:

The linked function accepts an address argument yet does not properly sanitize it.

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
65constructor(
66    ICLPoolManager _poolManager,
67    FeeConfig memory cfg_,
68    address _governance,
69    address _feeCollector
70) CLBaseHook(_poolManager) {
71    _updateFeeConfig(cfg_);
72    require(_governance != address(0), "Invalid governance");
73    governance = _governance;
74    feeCollector = _feeCollector;
75    // vault = IVault(_poolManager.vault());
76}

Recommendation:

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

Alleviation (5dd8944ae71b382923f6971e9985eb7f427147ec):

The input _poolManager address argument of the DynamicFeeHook::constructor function is adequately sanitized as non-zero in the latest in-scope revision of the codebase, addressing this exhibit.

DFH-03S: Potential Lock of Native Assets

Type	Severity	Location
Language Specific		DynamicFeeHook.sol:L201

Description:

The linked receive / fallback function performs no sanitization as to its caller and no function within the contract expects funds to have been received directly by the contract.

Impact:

Any native funds accidentally sent to the contract may be forever locked.

Example:

Copy
201receive() external payable {}

Recommendation:

We advise the code to properly prohibit accidental native assets from being permanently locked in the contract by introducing a require check restricting the msg.sender to the contract(s) expected to transfer assets to the system (i.e. in case of a wrapped native version of an asset, only the WXXX contract address should be allowed). Alternatively, if the contract is not expected to receive native assets directly the function should be removed in its entirety.

Alleviation (5dd8944ae7):

The Steer Protocol team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

Alleviation (04cd0485d1):

The Steer Protocol team re-visited this particular exhibit and opted to address it, mandating that the sender of native funds to the DynamicFeeHook::receive function is the vault itself.

DFH-04S: Improper Invocation of EIP-20 transfer

Type	Severity	Location
Standard Conformity		DynamicFeeHook.sol:L197

Description:

The linked statement does 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
197currency.transfer(feeCollector, amount);

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.

Alleviation (5dd8944ae71b382923f6971e9985eb7f427147ec):

After discussing with the Steer Protocol team, we deduced that this exhibit was erroneously referring to an EIP-20 transfer when a library-based transfer is being executed instead.

As such, we consider this exhibit to be nullified.