Description

The executeStrategy function first increases the strategy's allowance to type(uint256).max before calling the strategy contract's execute function. After execution, it queries the current allowance and then decreases it by the queried amount.

However, when the allowance is set to type(uint256).max, the transferFrom function does not decrease the allowance. So the external call to query underlyingAsset.allowance is unnecessary, as we know the allowance will still be type(uint256).max.

function executeStrategy(address _strategy, bytes calldata data) external onlyManager {
    _checkIsStrategy(_strategy);
    underlyingAsset.safeIncreaseAllowance(address(_strategy), type(uint256).max);
    IStrategy(_strategy).execute(data);
    uint256 allowance = underlyingAsset.allowance(address(this), address(_strategy));
    underlyingAsset.safeDecreaseAllowance(address(_strategy), allowance);

    emit StrategyExecuted(_strategy, data);
}

Impact

The unnecessary external call to query the allowance consumes additional gas, which could be optimized to reduce transaction costs.

Recommendations

Replace the allowance query and dynamic decrease with a direct decrease of type(uint256).max.

Remediation