StrategyBase.explanation() cannot be overridden to intended mutability #453
Description
Lines of code
Vulnerability details
Impact
An implementation of explanation(), as inherited from StrategyBase.sol, cannot (possibly contrary to intentions) make state modifications. This implies that StrategyBase.sol may become useless as the intended base contract to inherit from.
Proof of Concept
StrategyBase.sol "is designed to be inherited by more complex strategies, which can then override its functions as necessary".
Its function explanation() is declared as pure:
/**
* @notice Currently returns a brief string explaining the strategy's goal & purpose, but for more complex
* strategies, may be a link to metadata that explains in more detail.
*/
function explanation() external pure virtual override returns (string memory) {
return "Base Strategy implementation to inherit from for more complex implementations";
}This means that any inheriting contract overriding this function also must be pure. However, an implementation might need a mutability of at least view. This is suggested by it's being declared view in IStrategy.sol. For example, the explanation of the strategy might want to incorporate the value of some variable in the strategy, rather than just being an immutable string.
There is a similar issue with sharesToUnderlying() and underlyingToShares(), both reported separately.
Recommended Mitigation Steps
Declare explanation() as the default nonpayable.
- function explanation() external pure virtual override returns (string memory) {
+ function explanation() external view virtual override returns (string memory) {Assessed type
Context