Lines of code

https://github.com/code-423n4/2024-07-basin/blob/main/src/functions/Stable2.sol#L114

Vulnerability details

Vulnerability Details

The calcReserve function allows rounding in favor of either the well or the user, which is not ideal for a DeFi protocol. The appropriate rounding behavior for a DeFi protocol should be to always round in favor of the protocol, rather than the individual user, to ensure the stability and integrity of the protocol's internal calculations and pricing mechanisms.

The rationale behind this is that DeFi protocols are designed to be self-sustaining and decentralized, with the protocol's interests aligned with the collective interests of all users.

Code

https://github.com/code-423n4/2024-07-basin/blob/main/src/functions/Stable2.sol#L114

/**
     * @notice Calculate x[i] if one reduces D from being calculated for reserves to D
     * Done by solving quadratic equation iteratively.
     * x_1**2 + x_1 * (sum' - (A*n**n - 1) * D / (A * n**n)) = D ** (n + 1) / (n ** (2 * n) * prod' * A)
     * x_1**2 + b*x_1 = c
     * x_1 = (x_1**2 + c) / (2*x_1 + b)
     * @dev This function has a precision of +/- 1,
     * which may round in favor of the well or the user.

The comments in the code indicate that the function has a precision of +/- 1, and it may round in favor of the well or the user.

for (uint256 i; i < 255; ++i) {
            prevReserve = reserve;
            reserve = _calcReserve(reserve, b, c, lpTokenSupply);
            // Equality with the precision of 1
            // scale reserve down to original precision
            if (reserve > prevReserve) {
                if (reserve - prevReserve <= 1) {
                    return reserve / (10 ** (18 - decimals[j]));
                }
            } else {
                if (prevReserve - reserve <= 1) {
                    return reserve / (10 ** (18 - decimals[j]));
                }
            }

Poc

This test case checks that the calcReserve function always rounds the result in favor of the protocol, even when the result is on the boundary of a whole number.

function test_calcReserve_roundingDirection() public {
    uint256[] memory reserves = new uint256[](2);
    _data = abi.encode(18, 18);

    reserves[0] = 100 * 1e18;
    reserves[1] = 100 * 1e18;
    uint256 lpTokenSupply = _function.calcLpTokenSupply(reserves, _data);
    reserves[0] = 0;
    reserves[1] = 100 * 1e18 + 1;
    uint256 reserve0 = _function.calcReserve(reserves, 0, lpTokenSupply, _data);
    assertEq(reserve0, 100 * 1e18);

    reserves[0] = 100 * 1e18 + 1;
    reserves[1] = 0;
    uint256 reserve1 = _function.calcReserve(reserves, 1, lpTokenSupply, _data);
    assertEq(reserve1, 100 * 1e18);
}

Mitigation

The calcReserve function should be modified to always round in favor of the protocol.

for (uint256 i; i < 255; ++i) {
        prevReserve = reserve;
        reserve = _calcReserve(reserve, b, c, lpTokenSupply);
        // Always round down to favor the protocol
        if (reserve > prevReserve) {
            return reserve / (10 ** (18 - decimals[j]));
        } else {
            return (reserve - 1) / (10 ** (18 - decimals[j]));
        }
    }
    revert("did not find convergence");
}

Assessed type

Math

code-423n4 / 2024-07-basin-validation

Incorrect Rounding Behaviour in calcReserve Function #95