code-423n4 / 2023-11-canto-findings

7 stars 6 forks source link

[H1] GetPriceAndFee is a Gas Bomb #481

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/1155tech-contracts/src/bonding_curve/LinearBondingCurve.sol#L23

Vulnerability details

Impact

​ Transactions ran out of gas

Analysis of the vulnerability

The function getPriceAndFee(args) at LinearBondingCurve.sol calls log2 in a for loop

function getPriceAndFee(uint256 shareCount, uint256 amount)
    external
    view
    override
    returns (uint256 price, uint256 fee)
{
    for (uint256 i = shareCount; i < shareCount + amount; i++) {
        uint256 tokenPrice = priceIncrease * i;
        price += tokenPrice;
        fee += (getFee(i) * tokenPrice) / 1e18; @audit gas bomb
    }
}

The getFee() implementation is

function getFee(uint256 shareCount) public pure override returns (uint256) {
    uint256 divisor;
    if (shareCount > 1) {
        divisor = log2(shareCount);
    } else {
        divisor = 1;
    }
    // 0.1 / log2(shareCount)
    return 1e17 / divisor;
}

First of all, log2 consumes and average of 749 gas per iteration.

My test concludes that the user has to pay 1,8 millon gas for buying 1k tokens !!

Secondly, this is highly unnecessary because the integer part of the logarithm of a number is not changing so often between consecutive numbers

The following table shows that

Value log2
2 1
3 1
4 2
7 2
8 3
15 3

In fact, if shareCount >> count (much higher than) it is likely that log2 is constant.

Recommended

Your need to rewrite getPriceAndFee(uint256 shareCount, uint256 amount).
A first option is to calculate the log of the first and the last element and use some number in between to approximate fees.

A more complex option is to use Yul (assembly).

As the implementation of log2 returns the most significant bit of the number you could write some code to track when MSB changes in the loop.

Assessed type

Loop

c4-pre-sort commented 11 months ago

minhquanym marked the issue as duplicate of #489

c4-judge commented 11 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 11 months ago

MarioPoneder marked the issue as grade-c