`BondCalculator.computeCvgExpected` will always sum 1 to the result even if the division is exact.

[hats-bug-reporter[bot]]

Github username: @aviggiano Submission hash (on-chain): 0x675f36f00cd9a8a5098b0d6003ff381cbf7d9d4ec14882562dfae8d87d8663d6 Severity: medium

Description:

Description

In BondCalculator.sol:69, the function computeCvgExpected will always sum 1 to the result even if the division is exact. ABDKMathQuad.div, by default, rounds the result down. However, when the division is exact, adding 1 to the result is not necessary, and will in fact cause the result to be 1 more than necessary.

For example, in order to round up the result of the integer division roundUp(79/20), one can sum 1 to the final result, which will be 4 = roundUp(79/20) == 79/20+1 == 3+1 == 4. However, if the division is exact, summing up does not mean the result will be rounded up, but rather it will be off by one: 4 = roundUp(80/20) != 80/20+1 == 4+1 == 5.

Attack scenario

An attacker could exploit this rounding issue in the event composedFunction is set to 1 (ln bonding computation) in order to gain a greater amount of cvgExpected than expected.

Proof of Concept

    ) internal pure returns (bytes16 cvgExpected) {
        bytes16 timeRatio = computeTimeRatio(durationFromStart, totalDuration);

        if (composedFunction == 0) {
            cvgExpected = ABDKMathQuad.sqrt(timeRatio);
        } else if (composedFunction == 1) {
            cvgExpected = ABDKMathQuad
                .ln(timeRatio)
                .div(ABDKMathQuad.ln(ABDKMathQuad.fromUInt(totalOutToken / 10 ** 18)))
                .add(ABDKMathQuad.fromUInt(1));
        } else if (composedFunction == 2) {
            cvgExpected = timeRatio.mul(timeRatio);
        } else {
            cvgExpected = timeRatio;
        }
        cvgExpected = cvgExpected.mul(ABDKMathQuad.fromUInt(TEN_POWER_6));
        cvgExpected = cvgExpected.mul(ABDKMathQuad.fromUInt(totalOutToken / (TEN_POWER_6)));
    }

Recommendation

To resolve this issue, it is recommended to remove the addition of 1 for when the composedFunction equals 1, and instead use the actual result of the division operation.

[shalbe-cvg]

Hello, Thanks a lot for your attention.

As per our documentation, this math formula is correct. This is the expected behavior.

We have so to consider this issue as Invalid.