Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Lack of Input Validation and Access Control in FundFlowController.updateVaultGroups

xidik

Summary

The updateVaultGroups function in the FundFlowController contract lacks proper input validation and access control. Malicious or incorrect inputs could lead to inconsistent states, unbonding incorrect vaults, or mismanagement of funds. Additionally, the function can be called by any address, which poses a significant security risk.

Vulnerability Details

In the FundFlowController contract, the updateVaultGroups function is defined as:

function updateVaultGroups() external {
uint256 curUnbondedGroup = curUnbondedVaultGroup;
uint256 nextUnbondedGroup = _getNextGroup(curUnbondedGroup, numVaultGroups);
​
// ... code omitted for brevity ...
​
operatorVCS.updateVaultGroups(
curGroupOpVaultsToUnbond,
curGroupOpVaultsTotalDepositRoom,
nextUnbondedGroup,
nextGroupOpVaultsTotalUnbonded
);
communityVCS.updateVaultGroups(
curGroupComVaultsToUnbond,
curGroupComVaultsTotalDepositRoom,
nextUnbondedGroup,
nextGroupComVaultsTotalUnbonded
);
​
timeOfLastUpdateByGroup[curUnbondedGroup] = uint64(block.timestamp);
curUnbondedVaultGroup = uint64(nextUnbondedGroup);
}
​

Lack of Access Control:

  • The function lacks an access control modifier like onlyOwner or onlyAuthorized.

  • Any external address can call updateVaultGroups, potentially manipulating the contract's state.

Lack of Input Validation:

  • The function relies on internal calculations and external contract calls to update critical state variables.

  • The parameters passed to operatorVCS.updateVaultGroups and communityVCS.updateVaultGroups are not validated within the FundFlowController contract.

In OperatorVCS, the updateVaultGroups function is:

function updateVaultGroups(
uint256[] calldata _curGroupVaultsToUnbond,
uint256 _curGroupTotalDepositRoom,
uint256 _nextGroup,
uint256 _nextGroupTotalUnbonded
) external onlyFundFlowController {
for (uint256 i = 0; i < _curGroupVaultsToUnbond.length; ++i) {
vaults[_curGroupVaultsToUnbond[i]].unbond();
}
​
vaultGroups[globalVaultState.curUnbondedVaultGroup].totalDepositRoom = uint128(_curGroupTotalDepositRoom);
globalVaultState.curUnbondedVaultGroup = uint64(_nextGroup);
totalUnbonded = _nextGroupTotalUnbonded;
}
​

Even though updateVaultGroups in OperatorVCS is restricted by onlyFundFlowController, the FundFlowController itself lacks access control, allowing unauthorized entities to call updateVaultGroups and manipulate vault groups.

Impact

  • Unauthorized or incorrect unbonding of vaults, leading to withdrawal delays or loss of staking rewards.

  • Incorrect updates to totalDepositRoom and totalUnbonded could disrupt accounting, leading to misallocation of funds.

  • Users may experience loss of funds or rewards due to mismanagement caused by improper state updates.

  • The overall integrity and reliability of the staking system could be compromised.

Tools Used

Manual code review.

Recommendations

  • Add an access control modifier to updateVaultGroups, such as onlyOwner or a custom modifier restricting access to authorized entities.

  • Validate all input parameters within updateVaultGroups to ensure they are within expected ranges.

  • Verify that vault indices in _curGroupVaultsToUnbond are valid and correspond to existing vaults.

  • Check that _curGroupTotalDepositRoom and _nextGroupTotalUnbonded are accurate and consistent with current state.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.