VaultController.sol : `liquidationFee` can be arbitrarily set to any value during contract creation.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/daae020331404647c661ab534d20093c875483e1/core/solidity/contracts/core/VaultController.sol#L508-L519

Vulnerability details

Impact

liquidationFee can be set to any arbitrary value during contract creation. This will affects the liquidators negatively till the function changeLiquidationFee is called.

user might be thinking that the liquidation fee could be well below the 1e18. But it could be some other value. since this will lead to loss of funds and the value can be changes by calling the function changeLiquidationFee setting the severity as medium.

Proof of Concept.

Vaultcontroller has the set of input parameters which are used during borrow and repay. One of the variable is liquidationFee which is used to in the function getLiquidationFee in order to calculate the liquation fee during the liquidation.

when we look at the variable liquidationFee inside the function changeLiquidationFee, its value should not go above the 1e18

  function changeLiquidationFee(uint192 _newLiquidationFee) external override onlyOwner {
    if (_newLiquidationFee >= 1e18) revert VaultController_FeeTooLarge(); ---------------->> audit find
    uint192 _oldLiquidationFee = liquidationFee;
    liquidationFee = _newLiquidationFee;

    emit ChangedLiquidationFee(_oldLiquidationFee, _newLiquidationFee);

but when we look at the constructor during contract creation,

  constructor(
    IVaultController _oldVaultController,
    address[] memory _tokenAddresses,
    IAMPHClaimer _claimerContract,
    IVaultDeployer _vaultDeployer,
    uint192 _initialBorrowingFee,
    address _booster,
    uint192 _liquidationFee
  ) {
    VAULT_DEPLOYER = _vaultDeployer;
    interest = Interest(uint64(block.timestamp), 1 ether);
    protocolFee = 1e14;
    initialBorrowingFee = _initialBorrowingFee;
    liquidationFee = _liquidationFee; ------------->> audit find .. check is missing.

    claimerContract = _claimerContract;

    BOOSTER = IBooster(_booster);

    if (address(_oldVaultController) != address(0)) _migrateCollateralsFrom(_oldVaultController, _tokenAddresses);
  }

Tools Used

Manual review

Recommended Mitigation Steps

During contract creation, enforce the check that is set inside the function changeLiquidationFee. so that the liquidationFee is within the range.

Assessed type

Rug-Pull

[c4-pre-sort]

minhquanym marked the issue as duplicate of #24

[c4-judge]

dmvt marked the issue as unsatisfactory: Overinflated severity