code-423n4 / 2023-12-ethereumcreditguild-findings

17 stars 11 forks source link

Extending distribution period , delays distribution of unminted rebase rewards #1209

Closed c4-bot-2 closed 9 months ago

c4-bot-2 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/ERC20RebaseDistributor.sol#L364-L374

Vulnerability details

Summary

distribute credit tokens extends the distribution period, this leads to delay distribution of unminted rebase rewards, which is not preferred and can harm some users.

Impact

improper calculation of rebase rewards

Proof of Concept

function testExtendDistributionPeriod() public {

    // initial state: 2 addresses rebasing, both of them have 100 tokens
        vm.prank(alice);
        token.enterRebase();
        vm.prank(bobby);
        token.enterRebase();
        token.mint(alice, 100);
        token.mint(bobby, 100);

        // distribute 120 profits and then jump to targetTimestamp( end of the distribution period ) 
        token.mint(address(this), 120);
        token.approve(address(token), 120);
        token.distribute(120);
        // t1 = first distribution time
        uint256 t1 = block.timestamp;
        // t2 = middle of t1 and end of distribution period , where we perform second distribution 
        uint256 t2 = block.timestamp + token.DISTRIBUTION_PERIOD()/2;
        // t3 = end of the distribution period (first distribution)
        uint256 t3 = block.timestamp + token.DISTRIBUTION_PERIOD();
        vm.warp(t3);

        // as we expected after the distribution period at t3, each rebasing address has 160 tokens 
        // in this scenario no one calls distribute from last distribution 
        // scenario 1 : balance in t3 = 160 
        assertEq(token.balanceOf(alice), 160);
        assertEq(token.balanceOf(bobby), 160);

        // now consider a scenario where a user perform distribute before end of the distribution period  
        // now we go back sometime ago to t2 (in the middle, between start and end of the distribution period) 
        // in this scenario someone distributes 12 token at this time
        vm.warp(t2);
        token.mint(address(this), 12);
        token.approve(address(token), 12);  
        token.distribute(12);

        // now we go forward to t3
        // as we see every address has 148  tokens, unlike the first scenario 
        // If we don't perform distribution at t2 
        // scenario 2 : balance = 148
        // this clearly demonstrates that extending distribution period delays token distribution which is not preferred since some users may exit rebase early and they don't get all the distributed rewards in their holding time . 
        vm.warp(block.timestamp + token.DISTRIBUTION_PERIOD()/2);

        assertEq(token.balanceOf(alice), 148);
        assertEq(token.balanceOf(bobby), 148);
    })

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using a distribution rate for calculating rebase rewards, and adjust rate based on the distribution amount in different periods.

Assessed type

Other

c4-pre-sort commented 9 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 9 months ago

0xSorryNotSorry marked the issue as duplicate of #1100

c4-judge commented 8 months ago

Trumpero marked the issue as satisfactory