sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

KingNFT - A ````Game```` player will suffer loss while calling ````rebalanceBasket()```` between ````step 6```` and ````step 8```` #412

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

KingNFT

medium

A Game player will suffer loss while calling rebalanceBasket() between step 6 and step 8

Summary

While step 6 pushAllocationsToVaults() is end, isXChainRebalancing is been set to false, but settleRewardsInt() is called untill step 8. A Game player will suffer loss while calling rebalanceBasket() during this period.

Vulnerability Detail

As shown on L324 of Game contract, rebalanceBasket() can be called while isXChainRebalancing == false which is set on L476 of pushAllocationsToVaults(). But rewardPerLockedToken of current rebalance period will be 0 until settleRewardsInt() is called.

File: derby-yield-optimiser\contracts\Game.sol
318:   function rebalanceBasket(
319:     uint256 _basketId,
320:     int256[][] memory _deltaAllocations
321:   ) external onlyBasketOwner(_basketId) nonReentrant {
322:     uint256 vaultNumber = baskets[_basketId].vaultNumber;
323:     for (uint k = 0; k < chainIds.length; k++) {
324:       require(!isXChainRebalancing[vaultNumber][chainIds[k]], "Game: vault is xChainRebalancing");
325:     }
326: 
327:     addToTotalRewards(_basketId);
328:     int256 totalDelta = settleDeltaAllocations(_basketId, vaultNumber, _deltaAllocations);
329: 
330:     lockOrUnlockTokens(_basketId, totalDelta);
331:     setBasketTotalAllocatedTokens(_basketId, totalDelta);
332:     setBasketRebalancingPeriod(_basketId, vaultNumber);
333:   }

File: derby-yield-optimiser\contracts\Game.sol
465:   function pushAllocationsToVaults(uint256 _vaultNumber, uint32 _chain) external payable {
466:     address vault = getVaultAddress(_vaultNumber, _chain);
467:     require(vault != address(0), "Game: not a valid vaultnumber");
468:     require(isXChainRebalancing[_vaultNumber][_chain], "Vault is not rebalancing");
469: 
470:     int256[] memory deltas = protocolAllocationsToArray(_vaultNumber, _chain);
471: 
472:     IXProvider(xProvider).pushProtocolAllocationsToVault{value: msg.value}(_chain, vault, deltas);
473: 
474:     emit PushProtocolAllocations(_chain, getVaultAddress(_vaultNumber, _chain), deltas);
475: 
476:     isXChainRebalancing[_vaultNumber][_chain] = false;
477:   }

File: derby-yield-optimiser\contracts\Game.sol
512:   function settleRewardsInt(
513:     uint256 _vaultNumber,
514:     uint32 _chainId,
515:     int256[] memory _rewards
516:   ) internal {
517:     uint256 rebalancingPeriod = vaults[_vaultNumber].rebalancingPeriod;
518: 
519:     for (uint256 i = 0; i < _rewards.length; i++) {
520:       int256 lastReward = getRewardsPerLockedToken(
521:         _vaultNumber,
522:         _chainId,
523:         rebalancingPeriod - 1,
524:         i
525:       );
526:       vaults[_vaultNumber].rewardPerLockedToken[_chainId][rebalancingPeriod][i] =
527:         lastReward +
528:         _rewards[i];
529:     }
530:   }

If the basket owner call rebalanceBasket() during this period, will suffer loss.

Impact

A Game player will suffer loss while calling rebalanceBasket() between step 6 and step 8.

Code Snippet

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Game.sol#L318

Tool used

Manual Review

Recommendation

Don't allow users to call rebalanceBasket() during this period