Missing rebalance-status check in updateBitFlag()
leads to incorrect rebalancing
Description
The updateBitFlag()
function lacks a rebalance-status check.
function updateBitFlag(address basket, uint256 bitFlag) external onlyRole(_TIMELOCK_ROLE) {
// Checks
// Check if basket exists
uint256 indexPlusOne = _bmStorage.basketTokenToIndexPlusOne[basket];
if (indexPlusOne == 0) {
revert BasketTokenNotFound();
}
uint256 currentBitFlag = BasketToken(basket).bitFlag();
if (currentBitFlag == bitFlag) {
revert BitFlagMustBeDifferent();
}
// Check if the new bitFlag is inclusive of the current bitFlag
if ((currentBitFlag & bitFlag) != currentBitFlag) {
revert BitFlagMustIncludeCurrent();
}
address strategy = BasketToken(basket).strategy();
if (!WeightStrategy(strategy).supportsBitFlag(bitFlag)) {
revert BitFlagUnsupportedByStrategy();
}
bytes32 newId = keccak256(abi.encodePacked(bitFlag, strategy));
if (_bmStorage.basketIdToAddress[newId] != address(0)) {
revert BasketIdAlreadyExists();
}
// Remove the old bitFlag mapping and add the new bitFlag mapping
bytes32 oldId = keccak256(abi.encodePacked(currentBitFlag, strategy));
_bmStorage.basketIdToAddress[oldId] = address(0);
_bmStorage.basketIdToAddress[newId] = basket;
_bmStorage.basketAssets[basket] = AssetRegistry(_bmStorage.assetRegistry).getAssets(bitFlag);
emit BasketBitFlagUpdated(basket, currentBitFlag, bitFlag, oldId, newId);
// Update the bitFlag in the BasketToken contract
BasketToken(basket).setBitFlag(bitFlag);
}
The updateBitFlag()
function modifies the basketAssets
array using the following line:
_bmStorage.basketAssets[basket] = AssetRegistry(_bmStorage.assetRegistry).getAssets(bitFlag);
This behavior can lead to conflicts during rebalancing if an admin changes assets using updateBitFlag()
.
address[] memory assets = self.basketAssets[basket];
// nosemgrep: solidity.performance.array-length-outside-loop.array-length-outside-loop
uint256 assetsLength = assets.length;
basketBalances[i] = new uint256[](assetsLength);
for (uint256 j = 0; j < assetsLength;) {
address asset = assets[j];
// nosemgrep: solidity.performance.state-variable-read-in-a-loop.state-variable-read-in-a-loop
uint256 currentAssetAmount = self.basketBalanceOf[basket][asset];
basketBalances[i][j] = currentAssetAmount;
// nosemgrep: solidity.performance.state-variable-read-in-a-loop.state-variable-read-in-a-loop
totalValue_[i] += self.eulerRouter.getQuote(currentAssetAmount, asset, _USD_ISO_4217_CODE);
unchecked {
// Overflow not possible: j is less than assetsLength
++j;
}
}
The rebalancing logic, which depends on the basketAssets
array, may fetch incorrect or outdated assets. This leads to errors in balance calculations or rebalancing outcomes. Here is an error scenario.
The
updateBitFlag()
function is called during an active rebalance.The
basketAssets
array is updated with a new set of assets via thegetAssets(bitFlag)
call.The rebalancing logic uses the modified
basketAssets
array, potentially fetching incorrect assets:This results in incorrect balances, total value calculations, or improper asset rebalancing.
Recommendations
Add a rebalance-status check to updateBitFlag()
to prevent the function from being called while a rebalance is in progress.
Impact
If the updateBitFlag()
function is called during a rebalance, it may modify the basketAssets
array, leading to incorrect asset selection or balance calculation, potentially causing improper rebalancing and errors.
Remediation
This issue has been acknowledged by Storm Labs, and a fix was implemented in commit 425e4f9f↗.