c4-bot-8 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/ERC20TokenEmitter.sol#L105 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/ERC20TokenEmitter.sol#L271-L281

Vulnerability details

Impact

ERC20TokenEmitter is an upgradeable contract where it can be initialized/upgraded by the RevolutionBuilder contract, and when the contract is upgradeded; a new value for the startTime variable is set to equal the current block.timestamp.
startTime value is used to calculate the timeSinceStart that is used to evaluate the price of the governance ERC20 token via VRGDAC contract, and this price starts low and increases if the emitted (bought) token amounts is high (higher demand than the amount set to be emitted per time unit which is set to be one day), and decreses if the demand is low (behind amount to be emitted per time unit which is set to be one day).
So upgrading the contract will change the startTime, while the emittedTokenWad value (which represents the amount of sold tokens) will be preserved, and this will result in rising the price of the token with each upgrade as the demand will be conceived to be high within a short time (as emittedTokenWad doesn't represent the actual amount of tokens sold within block.timestamp - startTime period).

Proof of Concept

ERC20TokenEmitter.initialize function/L105

startTime = block.timestamp;

ERC20TokenEmitter.getTokenQuoteForPayment function

   function getTokenQuoteForPayment(uint256 paymentAmount) external view returns (int gainedX) {
        require(paymentAmount > 0, "Payment amount must be greater than 0");
        // Note: By using toDaysWadUnsafe(block.timestamp - startTime) we are establishing that 1 "unit of time" is 1 day.
        // solhint-disable-next-line not-rely-on-time
        return
            vrgdac.yToX({
                timeSinceStart: toDaysWadUnsafe(block.timestamp - startTime),
                sold: emittedTokenWad,
                amount: int(((paymentAmount - computeTotalReward(paymentAmount)) * (10_000 - creatorRateBps)) / 10_000)
            });
    }

Tools Used

Manual Review.

Recommended Mitigation Steps

Preserve the previous value of startTime when re-initializing the contract:

    function initialize(
        address _initialOwner,
        address _erc20Token,
        address _treasury,
        address _vrgdac,
        address _creatorsAddress
    ) external initializer {
        require(msg.sender == address(manager), "Only manager can initialize");

        __Pausable_init();
        __ReentrancyGuard_init();

        require(_treasury != address(0), "Invalid treasury address");

        // Set up ownable
        __Ownable_init(_initialOwner);

        treasury = _treasury;
        creatorsAddress = _creatorsAddress;
        vrgdac = VRGDAC(_vrgdac);
        token = NontransferableERC20Votes(_erc20Token);
-       startTime = block.timestamp;
+       if(startTime == 0) startTime = block.timestamp;
    }