Sandbox Sand Reward Pool

The Sandbox has an `anti-compound` mechanism to "implement a time buffer for reward retrieval". The associated modifier is defined as follow: ```solidity modifier antiCompoundCheck(address account) { require( block.timestamp > antiCompound.lastWithdraw[account] + antiCompound.lockPeriodInSecs, "SandRewardPool: must wait" ); antiCompound.lastWithdraw[account] = block.timestamp; _; } ``` The issue is that the variable `antiCompound.lastWithdraw[account]` is not set when a user enters `stake()`, meaning a user does not have to respect the delay when he first enters the staking contract before 7 days. Below is an example of a scenario that would bypass the mechanism, with `antiCompound.lockPeriodInSecs : 10 080 (7 days)`. Exploit Scenario: 1. User enters the contract with `stake()`. \ Since `antiCompound.lastWithdraw[account]` is not defined, it is equal to 0. 2. After 2 days, user calls `exit()`, since `antiCompound.lastWithdraw[account]` is 0, the condition below is met, and the user can exit the staking contract. ```solidity require(block.timestamp > antiCompound.lockPeriodInSecs,"SandRewardPool: must wait"); ``` 3. The user sends his staking token to a new address, and go back to Step 1. \ The user needs to use a new address since his current address now has a correct value for `antiCompound.lastWithdraw[account]`.

The Sandbox wants to ensure that people stake their tokens for a certain amount of time before being able to withdraw their rewards. Users can claim their rewards by : - Calling the `getReward()` function; - Exiting the staking feature with the `exit()` function. In both cases, the function to withdraw the rewards is `_withdrawRewards()` : ```solidity function _withdrawRewards(address account) internal antiCompoundCheck(account) { ``` This `_withdrawRewards()` function uses the `antiCompoundCheck` modifier: ```solidity modifier antiCompoundCheck(address account) { require( block.timestamp > antiCompound.lastWithdraw[account] + antiCompound.lockPeriodInSecs, "SandRewardPool: must wait" ); antiCompound.lastWithdraw[account] = block.timestamp; _; } ``` The modifier essentially ensures that a period of time is respected between 2 rewards withdrawals. The issue is that in case of major security issue regarding the project, users might need to exit the staking immediately with their rewards. \ However, if a user does not meet the previous condition from the modifier, the whole `exit()` transaction would revert and the user would be stuck. The auditors would like to know how The Sandbox team would deal this issue.

The staking contract operates as the main entry for interaction with staking users. The staking users deposit tokens into the `SandRewardPool` contract and in return get a proportionate amount of shares. Later on, the staking users can withdraw their own assets from the pool. In this procedure, `_stake()` and `_unstake()` from `StakeTokenWrapper` are involved in transferring users’ assets into (or out of) the protocol. When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged (and burned) transaction fee. As a result, this may not meet the assumption behind these low-level asset-transferring routines and will bring unexpected balance inconsistencies.

The given input are missing the check for the non-zero address: - `SandRewardPool`.`setTrustedForwarder()`: Modify the `Trusted Forwarder`; - `SandRewardPool`.`recoverFunds()`: Send rewards tokens from the contract.

The variable `rewardPool` assigned in the constructor can declare with `Immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since will not be stored in storage. Still, values will be directly inserted the values into the runtime code.