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

3 stars 2 forks source link

Emission Schedule Not Respected #668

Closed c4-bot-6 closed 10 months ago

c4-bot-6 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/ERC20TokenEmitter.sol#L254-L264 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/libs/VRGDAC.sol#L54-L83

Vulnerability details

In the VRGDAC contract, targetPrice is the target price for a token if sold at the expected rate. perTimeUnit indicates the number of tokens intended to be sold in one full unit of time. Essentially, a VRGDA contract dynamically adjusts the token's price to maintain a specific issuance schedule. If the emissions are ahead of schedule, the price should increase exponentially. Conversely, if behind schedule, the token's price should decrease at a constant decay rate.

The VRGDAC is designed to always exponentially increase the token's price if the supply exceeds the scheduled amount.

However, tests indicate that the supply can be ahead of schedule with the price falling below the target price.

Impact

Main invariant not respected : The VRGDAC should always exponentially increase the price of tokens if the supply is ahead of schedule.

The price to issuance schedule of the governance token is not working as intended.

Proof of Concept

The parameters are consistent with those in ERC20TokenEmitterTest:

More specifically, we have a targetPrice of 1 ether and a perTimeUnit of 1,000 tokens per day:

int256 oneFullTokenTargetPrice = 1 ether; // targetPrice
uint256 tokensPerTimeUnit = 1_000;        // Schedule is 1,000 tokens sold per day

1) In the following example, Alice buys 10,000 ether worth of ERC20TokenEmitter. The schedule is 1,000/day, meaning Alice is purchasing ten times the daily scheduled issuance. 2) After seven days, the supply remains ahead of schedule. 3) Bob then buys 1 ether worth of ERC20TokenEmitter and receives more than 1e18 ERC20TokenEmitter tokens, indicating that the price has dropped below the targetPrice.

    function testBuyTokenPriceAfter7days() public {
        vm.startPrank(address(0));

        address[] memory firstRecipients = new address[](1);
        firstRecipients[0] = address(1);

        address[] memory secondRecipients = new address[](1);
        secondRecipients[0] = address(2);

        uint256[] memory bps = new uint256[](1);
        bps[0] = 10_000;

        // alice
        erc20TokenEmitter.buyToken{ value: 10_000 ether }(
            firstRecipients,
            bps,
            IERC20TokenEmitter.ProtocolRewardAddresses({
                builder: address(0),
                purchaseReferral: address(0),
                deployer: address(0)
            })
        );
        vm.warp(block.timestamp + (7 days));

        // bob
        erc20TokenEmitter.buyToken{ value: 1 ether }(
            secondRecipients,
            bps,
            IERC20TokenEmitter.ProtocolRewardAddresses({
                builder: address(0),
                purchaseReferral: address(0),
                deployer: address(0)
            })
        );

        // should still be more expensive than the target price
        // but bob get more than one full token
        assertGt(erc20TokenEmitter.balanceOf(address(2)), 1 ether);

    }

Tools Used

Tests

Recommended Mitigation Steps

Address the mathematical discrepancies.

Assessed type

Math

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #404

c4-judge commented 9 months ago

MarioPoneder marked the issue as unsatisfactory: Invalid

c4-judge commented 9 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

MarioPoneder marked the issue as grade-c