sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

ak1 - MainVault.sol : sendRewardsToGame is harmed for re-entrancy attack #341

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ak1

high

MainVault.sol : sendRewardsToGame is harmed for re-entrancy attack

Summary

MainVault.sol#L361 - sendRewardsToGame() , sends the rewards to game by collecting the rewards from rewardsToArray(). This is done when state is State.SendRewardsPerToken.

sendRewardsToGame is calling the pushRewardsToGame() inXProvider.sol. Here, rewards are sent based on chain type(home or foreign)

When rewards is sent to home chain, it is updated in mapping(uint32 => mapping(uint256 => mapping(uint256 => int256))) rewardPerLockedToken; in the function settleRewardsInt.

If it is in another chain, the control is given outside which is making possibility for re-entrancy.

Note : State.Idle is set after calling the pushRewardsToGame

Vulnerability Detail

Refer the summary section.

Impact

  1. accumulation of rewards to one specify vault. This eventually lead to loss of the fund in the contract. All funds will be as rewards.
  2. Loss of funds if it is going outside of home chain. Again draining out the funds.

Code Snippet

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/MainVault.sol#L361-L370

Tool used

Manual Review

Recommendation

  1. Add non - renetrant modifier.
  2. set state = State.Idle; after checking for State.SendRewardsPerToken