Description

In the contracts AvonPool and Vault, some functions wrap the function schedule of the contract TimelockController to conveniently schedule updates to the contract settings and perform some sanity checks, such as the functions updateFlashLoanFee and updateManagerFees. However, for some functions that are scheduled to be called in these wrapper functions, the corresponding checks are not executed, for example in the following code related to updating the flashLoanFee.

function updateFlashLoanFee(
    uint64 newFee
) external onlyRole(PROPOSER_ROLE) {
!    if (newFee > PoolConstants.MAX_FLASH_LOAN_FEE) revert PoolErrors.InvalidInput();
    
    // [...]

    emit PoolEvents.FlashLoanFeeUpdateScheduled(address(this), newFee);
}

// Internal function that will be called by the timelock
function _executeUpdateFlashLoanFee(
    uint64 newFee
) external {
    if (msg.sender != address(this)) revert PoolErrors.Unauthorized();
    
    PoolStorage.PoolState storage s = PoolStorage._state();
    uint64 oldFee = s.flashLoanFee;
    s.flashLoanFee = newFee;
    
    emit PoolEvents.FlashLoanFeeUpdated(address(this), oldFee, newFee);
}

Impact

Since functions schedule and scheduleBatch of the contract TimelockController are still callable, proposers can directly call the function schedule or function scheduleBatch to schedule an update, thereby bypassing checks in the update wrapper functions and setting invalid values.

Recommendations

Consider performing sanity checks within the functions that execute updates.

Remediation