EverGrow EGC security record

In the `alterPackage` function, when `totalStaked` is 0, the `tokensPerSecond` value is recalculated using `endTime - block.timestamp`. However, `packages[packageId].lastUpdate` is not updated during this process. Since `packages[packageId].lastUpdate` remains unchanged, a user can stake and immediately claim rewards in the same transaction, right after the `alterPackage` function is called. When the first user stakes into the contract, the `stake` function calculates the `totalExcluded` field for this user within the package as follows: ```solidity packages[i].totalExcluded[user] = getTotalExcluded(user, i); ``` Immediately after staking, if the user calls the `unstake` function, it triggers the `_cleanupPackages` function: ```solidity uint256 timeSince = timeSinceUpdated(i); ... uint256 tokensPerSecond = packages[i].tokensPerSecond; // determine the number of tokens owed to holders uint256 amountOwed = tokensPerSecond * timeSince; // fetch difference between total distributed and total to distribute uint256 max_extra_rewards = packages[i].totalToDistribute - packages[i].totalDistributed; // if the amount owed is greater than this value, clamp it if (amountOwed > max_extra_rewards) { amountOwed = max_extra_rewards; } // increase dividendsPerShare by this value unchecked { packages[i].totalDistributed += amountOwed; packages[i].dividendsPerShare += (amountOwed * PRECISION) / totalStaked; } // update timing on package packages[i].lastUpdate = block.timestamp; unchecked { ++i; } } ``` In this process, `tokensPerSecond` is recalculated in the `alterPackage` function as `(totalDistribute - totalDistributed) / (endTime - block.timestamp)`. Since `timeSince` equals `block.timestamp - lastUpdate`, which is non-zero, the `amountOwed` (calculated as `tokensPerSecond * timeSince`) also results in a non-zero value. Consequently, `dividendsPerShare` increases by this non-zero `amountOwed`. When calculating the pending rewards: ```solidity function pendingRewards( address user, uint256 packageId ) public view returns (uint256) { if (userInfo[user].amount == 0) { return 0; } uint256 userExcluded = packages[packageId].totalExcluded[user]; uint256 currentNonExcluded = getTotalExcluded(user, packageId); ... uint256 pending = currentNonExcluded <= userExcluded ? 0 : currentNonExcluded - userExcluded; return pending > remainingDistribution ? remainingDistribution : pending; } ``` Due to the increase in `dividendsPerShare`, `currentNonExcluded` is guaranteed to be greater than `userExcluded`, which was calculated during the `stake` function. As a result, if the admin alters the packages when `totalStaked` is 0, the first staker is able to stake and claim rewards from this contract within the same transaction.

In the contract `EverGrowStaking`, the role `owner` has the authority to update the status of the `setPause` function and further pause/resume the functionality of the `unstake` function. Any compromise to the private key of the `owner` may allow hackers to take advantage of this authority and prevent external user access to the function `unstake` to withdraw the staking tokens.

In the `alterPackage` function, when `totalStaked` is 0, the `tokensPerSecond` value is recalculated using `endTime - block.timestamp`. However, `packages[packageId].lastUpdate` is not updated during this process. Since `packages[packageId].lastUpdate` remains unchanged, a user can stake and immediately claim rewards in the same transaction, right after the `alterPackage` function is called. When the first user stakes into the contract, the `stake` function calculates the `totalExcluded` field for this user within the package as follows: ```solidity packages[i].totalExcluded[user] = getTotalExcluded(user, i); ``` Immediately after staking, if the user calls the `unstake` function, it triggers the `_cleanupPackages` function: ```solidity uint256 timeSince = timeSinceUpdated(i); ... uint256 tokensPerSecond = packages[i].tokensPerSecond; // determine the number of tokens owed to holders uint256 amountOwed = tokensPerSecond * timeSince; // fetch difference between total distributed and total to distribute uint256 max_extra_rewards = packages[i].totalToDistribute - packages[i].totalDistributed; // if the amount owed is greater than this value, clamp it if (amountOwed > max_extra_rewards) { amountOwed = max_extra_rewards; } // increase dividendsPerShare by this value unchecked { packages[i].totalDistributed += amountOwed; packages[i].dividendsPerShare += (amountOwed * PRECISION) / totalStaked; } // update timing on package packages[i].lastUpdate = block.timestamp; unchecked { ++i; } } ``` In this process, `tokensPerSecond` is recalculated in the `alterPackage` function as `(totalDistribute - totalDistributed) / (endTime - block.timestamp)`. Since `timeSince` equals `block.timestamp - lastUpdate`, which is non-zero, the `amountOwed` (calculated as `tokensPerSecond * timeSince`) also results in a non-zero value. Consequently, `dividendsPerShare` increases by this non-zero `amountOwed`. When calculating the pending rewards: ```solidity function pendingRewards( address user, uint256 packageId ) public view returns (uint256) { if (userInfo[user].amount == 0) { return 0; } uint256 userExcluded = packages[packageId].totalExcluded[user]; uint256 currentNonExcluded = getTotalExcluded(user, packageId); ... uint256 pending = currentNonExcluded <= userExcluded ? 0 : currentNonExcluded - userExcluded; return pending > remainingDistribution ? remainingDistribution : pending; } ``` Due to the increase in `dividendsPerShare`, `currentNonExcluded` is guaranteed to be greater than `userExcluded`, which was calculated during the `stake` function. As a result, if the admin alters the packages when `totalStaked` is 0, the first staker is able to stake and claim rewards from this contract within the same transaction.

In the contract `EverGrowStaking`, the role `owner` has the authority to update the status of the `setPause` function and further pause/resume the functionality of the `unstake` function. Any compromise to the private key of the `owner` may allow hackers to take advantage of this authority and prevent external user access to the function `unstake` to withdraw the staking tokens.

The `endPackage` function allows the `admin` role to terminate both active and expired packages. When determining the time elapsed since the last update, it invokes the `timeSinceUpdated(packageId)` function: ```solidity function timeSinceUpdated(uint256 packageId) public view returns (uint256) { if (packageId >= packageNonce || packageHasEnded(packageId)) { return 0; } ... } ``` If a package has expired, `packageHasEnded(packageId)` will return `true`, as `packages[packageId].endTime <= block.timestamp`: ```solidity function packageHasEnded(uint256 packageId) public view returns (bool) { return packages[packageId].endTime <= block.timestamp || packages[packageId].totalDistributed >= packages[packageId].totalToDistribute || packages[packageId].hasEnded; } ``` Given the logic above, `timeSinceUpdated(packageId)` will return `0` for an expired package. Consequently, in the following code snippet, `amountOwed` is calculated as `0`. This means that the `dividendsPerShare` for the expired package is not updated. As a result, any rewards that should have been accrued between `packages[packageId].lastUpdate` and `packages[packageId].endTime` will not be reflected in the `dividendsPerShare` field. Therefore, stakers who had staked when the package was active but unstaked after the package expired and was ended by the admin will receive no rewards from this package. ```solidity uint256 tSince = timeSinceUpdated(packageId); uint256 amountOwed = tSince * packages[packageId].tokensPerSecond; // fetch difference between total distributed and total to distribute uint256 max_extra_rewards = packages[packageId].totalToDistribute - packages[packageId].totalDistributed; // if the amount owed is greater than this value, clamp it if (amountOwed > max_extra_rewards) { amountOwed = max_extra_rewards; } // alter staking info for package, if and only if there are stakers if (totalStaked > 0) { // increase dividendsPerShare by this value unchecked { packages[packageId].totalDistributed += amountOwed; packages[packageId].dividendsPerShare += (amountOwed * PRECISION) / totalStaked; } ... ``` We would like to discuss this finding with the team to ensure that this design is intentional and correct.

The `_claim` function uses `IERC20(packages[packageId].rewardToken).balanceOf(address(this))` to determine the total reward token balance held by the contract. It then compares this balance with the user's pending reward (`pending`) to ensure that the pending reward does not exceed the total reward balance transferred into the contract by the admin. ```solidity uint256 pending = pendingRewards(user, packageId); if (pending == 0) { return; } uint256 max_balance = IERC20(packages[packageId].rewardToken).balanceOf(address(this)); if (pending > max_balance) { pending = max_balance; } ``` However, if the `rewardToken` is the same as the staking token, then this balance represents both the reward tokens and the staking tokens. Using this balance to compare with the user's pending reward does not ensure that the pending reward is less than the actual reward token balance allocated by the package. As a result, some users may receive more reward tokens than expected from this contract, while other users might not be able to reclaim their staked tokens.

The `endPackage` function allows the `admin` role to terminate both active and expired packages. When determining the time elapsed since the last update, it invokes the `timeSinceUpdated(packageId)` function: ```solidity function timeSinceUpdated(uint256 packageId) public view returns (uint256) { if (packageId >= packageNonce || packageHasEnded(packageId)) { return 0; } ... } ``` If a package has expired, `packageHasEnded(packageId)` will return `true`, as `packages[packageId].endTime <= block.timestamp`: ```solidity function packageHasEnded(uint256 packageId) public view returns (bool) { return packages[packageId].endTime <= block.timestamp || packages[packageId].totalDistributed >= packages[packageId].totalToDistribute || packages[packageId].hasEnded; } ``` Given the logic above, `timeSinceUpdated(packageId)` will return `0` for an expired package. Consequently, in the following code snippet, `amountOwed` is calculated as `0`. This means that the `dividendsPerShare` for the expired package is not updated. As a result, any rewards that should have been accrued between `packages[packageId].lastUpdate` and `packages[packageId].endTime` will not be reflected in the `dividendsPerShare` field. Therefore, stakers who had staked when the package was active but unstaked after the package expired and was ended by the admin will receive no rewards from this package. ```solidity uint256 tSince = timeSinceUpdated(packageId); uint256 amountOwed = tSince * packages[packageId].tokensPerSecond; // fetch difference between total distributed and total to distribute uint256 max_extra_rewards = packages[packageId].totalToDistribute - packages[packageId].totalDistributed; // if the amount owed is greater than this value, clamp it if (amountOwed > max_extra_rewards) { amountOwed = max_extra_rewards; } // alter staking info for package, if and only if there are stakers if (totalStaked > 0) { // increase dividendsPerShare by this value unchecked { packages[packageId].totalDistributed += amountOwed; packages[packageId].dividendsPerShare += (amountOwed * PRECISION) / totalStaked; } ... ``` We would like to discuss this finding with the team to ensure that this design is intentional and correct.

The `_claim` function uses `IERC20(packages[packageId].rewardToken).balanceOf(address(this))` to determine the total reward token balance held by the contract. It then compares this balance with the user's pending reward (`pending`) to ensure that the pending reward does not exceed the total reward balance transferred into the contract by the admin. ```solidity uint256 pending = pendingRewards(user, packageId); if (pending == 0) { return; } uint256 max_balance = IERC20(packages[packageId].rewardToken).balanceOf(address(this)); if (pending > max_balance) { pending = max_balance; } ``` However, if the `rewardToken` is the same as the staking token, then this balance represents both the reward tokens and the staking tokens. Using this balance to compare with the user's pending reward does not ensure that the pending reward is less than the actual reward token balance allocated by the package. As a result, some users may receive more reward tokens than expected from this contract, while other users might not be able to reclaim their staked tokens.