Uploaded files for judging

001.md

@@ -0,0 +1,118 @@
+Helpful Dijon Spider
+
+Medium
+
+# Calling `SNStInit` just 1 block after `SNstDeploy` will always revert and deposits will be stuck getting no yield
+
+### Summary
+
+Not calling `SNst::drip()` before `SNst::file()` in `SNstInit::init()` will DoS the protocol until a manual fix is applied. Depositing at the same block that `SNst` is deployed will get the funds stuck getting no yield until the manual fix comes through.
+
+### Root Cause
+
+[SNst::init()](https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/sdai/deploy/SNstInit.sol#L62-L64) does not call `SNst::drip()` before `SNst::file()`:
+```solidity
+function init(
+    ...
+) internal {
+    ...
+    dss.vat.rely(instance.sNst);
+     //@audit note that SNst::drip() is not called, which reverts if block.timestamp != rho
+    SNstLike(instance.sNst).file("nsr", cfg.nsr);
+    ...
+}
+```
+So it reverts on [SNst::file()](https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/sdai/src/SNst.sol#L206):
+```solidity
+function file(bytes32 what, uint256 data) external auth {
+    ...
+        require(rho == block.timestamp, "SNst/chi-not-up-to-date");
+    ...
+}
+```
+Additionally, deposits can be made when `block.timestamp == rho`, as `SNst::drip()` does [not](https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/sdai/src/SNst.sol#L226) call `vat::suck()`, which is what makes it revert as `vat::rely()` has not yet been set. After 1 block, they can not withdraw because `block.timestamp != rho` and `SNst::drip()` calls [vat::suck()](https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/sdai/src/SNst.sol#L221), which reverts.
+```solidity
+function drip() public returns (uint256 nChi) {
+    ...
+    if (block.timestamp > rho_) {
+        ...
+        vat.suck(address(vow), address(this), diff * RAY); //@audit reverts before vat::rely() is called in SNstInit
+        ...
+    } else { //@audit when block.timestamp == rho it goes here, allowing deposits
+        nChi = chi_;
+    }
+    ...
+}
+```
+
+
+### Internal pre-conditions
+
+1. `SNstInit::init()` must not be called in the same block `SNstDeploy::deploy()` is called.
+
+### External pre-conditions
+
+None.
+
+### Attack Path
+
+1. `SNstDeploy::deploy()` deploys the token.
+2. `$NST` is deposited into `SNst` in the same block of the deployment.
+3. Unless `SNstInit::init()` is called in the same block as the previous 2 points, it will be impossible to withdraw and deposits will get no yield.
+
+### Impact
+
+Funds are stuck until a manual `Vat::rely()` call is made allowing `SNst::drip()` to be called which allows withdrawals. When this call happens some time in the future, yield will be lost because the `nsr` is initially set to `1`, so deposits will get no interest.
+
+### PoC
+
+Add the following test to `SNst-integration.t.sol`:
+```solidity
+function testStuckDeposit() public {
+    SNstInstance memory inst = SNstDeploy.deploy(address(this), pauseProxy, address(nstJoin));
+    token = SNst(inst.sNst);
+
+    address user = makeAddr("user");
+    vm.startPrank(user);
+    deal(address(nst), user, 100 ether);
+    nst.approve(address(token), type(uint256).max);
+    token.deposit(100 ether, user);
+    vm.stopPrank();
+
+    skip(1);
+
+    vm.startPrank(pauseProxy);
+    vm.expectRevert("SNst/chi-not-up-to-date");
+    token.file("nsr", 1000000001547125957863212448); // init lib would also revert
+    vm.stopPrank();
+
+    vm.startPrank(user);
+    vm.expectRevert("Vat/not-authorized"); // drip() fails as rely() was not called
+    token.redeem(100 ether, user, user);
+}
+```
+
+### Mitigation
+
+`SNst::init()` must call `SNst::drip()` before `SNst::file()`:
+```solidity
+function init(
+    ...
+) internal {
+    ...
+    dss.vat.rely(instance.sNst);
+    
+    SNstLike(instance.sNst).drip(); //@audit add this so SNst::file() does not revert
+
+    SNstLike(instance.sNst).file("nsr", cfg.nsr);
+    ...
+}
+```
+Additionally, deposits must not be allowed at the block `SNst::deploy()` is called, or they will be stuck until `SNst::init()` is executed, which could take some time. The easiest fix is to set `rho` 1 block in the past, such that deposits will revert on `SNst::drip()` when calling `vat::suck()` before `vat::rely()` has been called in `SNst::init()`:
+```solidity
+function initialize() initializer external {
+    ...
+    rho = uint64(block.timestamp - 1); //@audit this stops deposits before SNst::init() is called
+    ...
+}
+```

001/002.md

@@ -0,0 +1,77 @@
+Cuddly Inky Rat
+
+Medium
+
+# PrecisionLoss in NotifyRewardAmount
+
+## Summary
+The `notifyRewardAmount` function in the smart contract has an incorrect validation mechanism for the reward rate. The current implementation uses integer division to check if the reward rate is within the contract's balance, leading to potential precision loss and incorrect validation, especially over long durations like 7 days. This can cause the function to pass invalid reward rates, resulting in incorrect reward distributions.
+https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/synthetix/StakingRewards.sol#L144
+## Vulnerability Detail
+The require statement intended to validate the reward rate uses integer division, which truncates the result, leading to potential precision loss. This can result in an incorrect validation of the reward rate, particularly in scenarios where the division leads to unexpected results due to truncation.
+
+rewardsDuration = 7 days = 604800 seconds
+balance = 1000000 tokens
+rewardRate initially set to 0
+
+Let's say we want to set reward = 500000 tokens.
+```solidity
+rewardRate = reward / rewardsDuration; // 500000 / 604800 ≈ 0.826 tokens per second
+```
+```solidity
+balance / rewardsDuration = 1000000 / 604800 ≈ 1.653 tokens per second (integer division truncates to 1)
+```
+rewardRate ≈ 0.826
+balance / rewardsDuration ≈ 1
+
+Since 0.826 <= 1, the require statement passes.
+
+Let assume a scenario where the balance is less than the RewardDuration due to reduction of balance and of that.
+Assume balance = 10000 tokens and rewardsDuration = 7 days = 604800 seconds.
+
+```solidity
+rewardRate = reward / rewardsDuration; // 4 / 604800 ≈ 0 (since 4 < 604800)
+```
+```solidity
+balance / rewardsDuration = 10 / 604800 ≈ 0 (integer division truncates to 0)
+```
+rewardRate = 0
+balance / rewardsDuration = 0
+
+Since 0 <= 0, the require statement passes. However, let's look at what happens in terms of rewards distribution:
+
+Total distributed reward: rewardRate * rewardsDuration = 0 * 604800 = 0 tokens.
+Given reward: 4 tokens.
+
+## Impact
+ Precision loss in the balance of tokens to be distributed to the stakers,  resulting in incorrect reward distributions.
+
+## Code Snippet
+```solidity
+function notifyRewardAmount(uint256 reward) external override onlyRewardsDistribution updateReward(address(0)) {
+    if (block.timestamp >= periodFinish) {
+        rewardRate = reward / rewardsDuration;
+    } else {
+        uint256 remaining = periodFinish - block.timestamp;
+        uint256 leftover = remaining * rewardRate;
+        rewardRate = (reward + leftover) / rewardsDuration;
+    }
+
+    // Ensure the provided reward amount is not more than the balance in the contract.
+    // This keeps the reward rate in the right range, preventing overflows due to
+    // very high values of rewardRate in the earned and rewardsPerToken functions;
+    // Reward + leftover must be less than 2^256 / 10^18 to avoid overflow.
+    uint256 balance = rewardsToken.balanceOf(address(this));
+    require(rewardRate <= balance / rewardsDuration, "Provided reward too high");
+
+    lastUpdateTime = block.timestamp;
+    periodFinish = block.timestamp + rewardsDuration;
+    emit RewardAdded(reward);
+}
+```
+## Tool used
+
+Manual Review
+
+## Recommendation
+You could accumulate the differences that occur due to precision/truncation and let users claim them  at the end and according to their shares

001/003.md

@@ -0,0 +1,112 @@
+Cuddly Inky Rat
+
+Medium
+
+# Loss of rewards when distribution occurs caused by # notifyRewardAmount
+
+## Summary
+The distribute function is responsible for distributing rewards that have accrued since the last distribution. It interacts with a vesting contract (dssVest) to fetch the amount of rewards that are due, then transfers these rewards to a staking rewards contract (stakingRewards). Finally, it notifies the staking rewards contract about the new reward amount.
+
+However the some part of the  rewards in the contract will locked due to precision loss and trauncations
+https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/VestedRewardsDistribution.sol#L152
+
+## Vulnerability Detail
+When `distirbution` occurs to the function calls stakingRewards.notifyRewardAmount(amount). This function in the StakingRewards contract adjusts the internal accounting to account for the new rewards. 
+
+In the notifyRewardAMount there is sought of precision loss, meaning users get to get less of their notified reward rather than expected, This can be seen here:
+```solidity
+ function notifyRewardAmount(uint256 reward) external override onlyRewardsDistribution updateReward(address(0)) {
+        if (block.timestamp >= periodFinish) {
+            rewardRate = reward / rewardsDuration;
+        } else {
+            uint256 remaining = periodFinish - block.timestamp;
+            uint256 leftover = remaining * rewardRate;
+            rewardRate = (reward + leftover) / rewardsDuration;
+        }
+
+        // Ensure the provided reward amount is not more than the balance in the contract.
+        // This keeps the reward rate in the right range, preventing overflows due to
+        // very high values of rewardRate in the earned and rewardsPerToken functions;
+        // Reward + leftover must be less than 2^256 / 10^18 to avoid overflow.
+        uint256 balance = rewardsToken.balanceOf(address(this));
+        require(rewardRate <= balance / rewardsDuration, "Provided reward too high");
+
+        lastUpdateTime = block.timestamp;
+        periodFinish = block.timestamp + rewardsDuration;
+        emit RewardAdded(reward);
+    }
+
+```
+How can this occur, if you look at the function logic you can see that there can be a precision loss because of the way the integer is been handled, Solidity only supports integer division, which can result in precision loss. This precision loss can become significant when dealing with reward rates and durations, causing the calculations to deviate from the expected values.
+
+The staking rewards contract is initialized with a `rewardsDuration` of 7 days (604800 seconds).
+The `periodFinish` is the timestamp when the current reward period ends.
+Initially, rewardRate is 0, and the contract has a balance of 1000 `rewardsToken`.
+
+Assume the current `periodFinish` is `July 1st, 2024`, and Alice adds a reward on `July 2nd, 2024` (after the current period has finished).
+Alice calls `notifyRewardAmount` with reward = 700.
+
+Since `block.timestamp` is after `periodFinish`, the condition `block.timestamp` >= `periodFinish` is true.
+The `rewardRate` is calculated as `reward` / `rewardsDuration`, which is 700 / 604800 = 0 (due to integer division).
+
+The balance in the contract is 1000 tokens.
+`balance / rewardsDuration` is 1000 / 604800 = 0 (due to integer division).
+The check `require(rewardRate <= 0)` is true since rewardRate is 0.
+The function proceeds without reverting.
+
+`lastUpdateTime` is updated to the current timestamp `(July 2nd, 2024).`
+`periodFinish` is set to July 9th, 2024.
+RewardAdded event is emitted with reward = 700.
+
+The rewardRate is set to 0 due to integer division (700 / 604800).
+This means no rewards will be distributed over the new period despite adding 700 tokens.
+
+
+## Impact
+This indicates a significant issue because the actual reward distribution  does not match the intended reward
+
+## Code Snippet
+```solidity
+function distribute() external returns (uint256 amount) {
+        require(vestId != INVALID_VEST_ID, "VestedRewardsDistribution/invalid-vest-id");
+
+        amount = dssVest.unpaid(vestId);
+        require(amount > 0, "VestedRewardsDistribution/no-pending-amount");
+
+        lastDistributedAt = block.timestamp;
+        dssVest.vest(vestId, amount);
+
+        require(gem.transfer(address(stakingRewards), amount), "VestedRewardsDistribution/transfer-failed");
+        stakingRewards.notifyRewardAmount(amount);
+
+        emit Distribute(amount);
+    }
+```
+```solidity
+ function notifyRewardAmount(uint256 reward) external override onlyRewardsDistribution updateReward(address(0)) {
+        if (block.timestamp >= periodFinish) {
+            rewardRate = reward / rewardsDuration;
+        } else {
+            uint256 remaining = periodFinish - block.timestamp;
+            uint256 leftover = remaining * rewardRate;
+            rewardRate = (reward + leftover) / rewardsDuration;
+        }
+
+        // Ensure the provided reward amount is not more than the balance in the contract.
+        // This keeps the reward rate in the right range, preventing overflows due to
+        // very high values of rewardRate in the earned and rewardsPerToken functions;
+        // Reward + leftover must be less than 2^256 / 10^18 to avoid overflow.
+        uint256 balance = rewardsToken.balanceOf(address(this));
+        require(rewardRate <= balance / rewardsDuration, "Provided reward too high");
+
+        lastUpdateTime = block.timestamp;
+        periodFinish = block.timestamp + rewardsDuration;
+        emit RewardAdded(reward);
+    }
+```
+
+## Tool used
+Manual Review
+
+## Recommendation
+Find a way to prevent precision loss, either by using safeMath because the notifyRewardAmount shouldn't be in anyway decreasing.

001/055.md

@@ -0,0 +1,40 @@
+Soft Turquoise Turtle
+
+Medium
+
+# StakingRewards: Significant loss of precision possible
+
+## Summary
+In notifyRewardAmount, the reward rate per second is calculated. This calculation rounds down, which can lead to situations where significantly less rewards are paid out to stakers, because the effect of the rounding is multiplied by the duration.
+## Vulnerability Detail
+ function notifyRewardAmount(uint256 reward) external override onlyRewardsDistribution updateReward(address(0)) {
+        if (block.timestamp >= periodFinish) {
+            rewardRate = reward / rewardsDuration;
+        } else {
+            uint256 remaining = periodFinish - block.timestamp;
+            uint256 leftover = remaining * rewardRate;
+            rewardRate = (reward + leftover) / rewardsDuration;
+        }
+
+        // Ensure the provided reward amount is not more than the balance in the contract.
+        // This keeps the reward rate in the right range, preventing overflows due to
+        // very high values of rewardRate in the earned and rewardsPerToken functions;
+        // Reward + leftover must be less than 2^256 / 10^18 to avoid overflow.
+        uint256 balance = rewardsToken.balanceOf(address(this));
+        require(rewardRate <= balance / rewardsDuration, "Provided reward too high");
+
+        lastUpdateTime = block.timestamp;
+        periodFinish = block.timestamp + rewardsDuration;
+        emit RewardAdded(reward);
+    }
+## Impact
+et's say we have a rewardsDuration of 4 years, i.e. 126144000 seconds. We assume the rewardRate is currently ß and notifyRewardAmount is called with the reward amount 252287999. Because the calculation rounds down, rewardRate will be 1. After the 4 years, the user have received 126144000 reward tokens. However, 126143999 (i.e., almost 50%) of the reward tokens that were intended to be distributed to the stakers were not distributed, resulting in monetary loss for all stakers.
+function notifyRewardAmount(uint256 _reward, uint256 _rewardUsdc)
+## Code Snippet
+https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/synthetix/StakingRewards.sol#L144C4-L163C6
+## Tool used
+
+Manual Review
+
+## Recommendation
+You could accumulate the differences that occur due to rounding and let the users claim them in the end according to their shares.