Values depending on the `VRGDAC` contract may be wrongly calculated due to possible precision loss.

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/libs/VRGDAC.sol#L54-L97 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/ERC20TokenEmitter.sol#L179-L184 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/ERC20TokenEmitter.sol#L237 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/ERC20TokenEmitter.sol#L254 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/ERC20TokenEmitter.sol#L271

Vulnerability details

Impact

The VRGDAC contract implements various cases of divsion before multiplication. Functions depending on these calculations may be calculated incorrectly. Wrong getTokenQuoteForEther value will be returned and the amount of tokens sold to the users will be incorrect or possibly 0. This can also be a source of accounting issues in the protocol.

Proof of Concept

The buyToken function calculates tokens to emit to creators and buyers by calling the getTokenQuoteForEther function.

        int totalTokensForCreators = ((msgValueRemaining - toPayTreasury) - creatorDirectPayment) > 0
            ? getTokenQuoteForEther((msgValueRemaining - toPayTreasury) - creatorDirectPayment) //@note 
            : int(0);

        // Tokens to emit to buyers
        int totalTokensForBuyers = toPayTreasury > 0 ? getTokenQuoteForEther(toPayTreasury) : int(0); //@note

This function then makes a call to the VRGAC contract, calling the yToX function.

    function getTokenQuoteForEther(uint256 etherAmount) public view returns (int gainedX) {
        require(etherAmount > 0, "Ether 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({ //@note
                timeSinceStart: toDaysWadUnsafe(block.timestamp - startTime),
                sold: emittedTokenWad,
                amount: int(etherAmount)
            });
    }

The yToX function implements divisions before multiplication, and is vulnerable to the precision loss.

function yToX(int256 timeSinceStart, int256 sold, int256 amount) public view virtual returns (int256) {
    int256 soldDifference = wadMul(perTimeUnit, timeSinceStart) - sold;
    unchecked {
        return
            wadMul(
                perTimeUnit,
                wadDiv(
                    wadLn(
                        wadDiv( //@note
                            wadMul( //@note
                                targetPrice,
                                wadMul( //@note
                                              ...
}

With large and small enough values, the function can return 0, causing that users receive no tokens for the amount paid. Other functions affected are the buyTokenQuote, getTokenQuoteForPayment and

Other functions affected are the

Tools Used

Manual Code Review

Recommended Mitigation Steps

Consider restructuring the functions to implement the multiplications first before the divisions.

Assessed type

Math

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #428

[c4-pre-sort]

raymondfam marked the issue as duplicate of #661

[c4-judge]

MarioPoneder marked the issue as not a duplicate

[c4-judge]

MarioPoneder changed the severity to QA (Quality Assurance)

[c4-judge]

MarioPoneder marked the issue as grade-b