beforeAction not surely called

[sherlock-admin3]

beforeAction not surely called

Low/Info issue submitted by petarP1998

Summary

The StrategyPassiveManagerVelodrome::beforeAction function is designed to be called during deposit and withdrawal operations to handle liquidity removal and fee harvesting for accounting purposes. However, there is no validation to ensure that beforeAction was actually invoked before deposit or withdraw.

Vulnerability Detail

The StrategyPassiveManagerVelodrome::beforeAction function, as per its natspec documentation, is intended to execute during deposit and withdrawal processes to manage liquidity and fees accurately. Despite this, the current implementation lacks a mechanism to confirm that beforeAction has been called immediately prior to deposit or withdraw. This oversight could potentially allow for improper sequencing of these calls, leading to inaccurate accounting.

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L197-L201

Impact

Without validation of the beforeAction call, there is a risk that liquidity and fee accounting may not be properly handled, which could result in financial discrepancies and errors in the contract's operation.

Code Snippet

/// @notice Called during deposit and withdraw to remove liquidity and harvest fees for accounting purposes.
function beforeAction() external {
    _onlyVault();
    _claimEarnings();
    _removeLiquidity();
}

Tool used

Manual Review

Recommendation

Implement a state variable to track the block number when StrategyPassiveManagerVelodrome::beforeAction was last called. Ensure that this block number is checked to be within a certain range of blocks when deposit and withdraw functions are called. This will enforce the correct sequence of operations and ensure accurate liquidity and fee management.

For example it could look like this:

...
uint256 public lastActionBlock;
uint32 const BLOCK_RANGE = 5;

function beforeAction() public {
    lastActionBlock = block.number; 
    ...
} 

function deposit() public { 
    require(block.number <= lastActionBlock + BLOCK_RANGE, "beforeAction not recently called"); 
    ...
} 

function withdraw() public { 
    require(block.number <= lastActionBlock + BLOCK_RANGE, "beforeAction not recently called"); 
    ...
}