discrepancy of _A max value in Curve and Thorn

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xb45179c8b4589c34d2cea8c73c7244e775154a89816bcec6086625b90b4379d8 Severity: low

Description: Description\ There is a discrepancy of the max value of the amplification coefficient _A, that can be set in Thorn versus Curve. This makes it possible for the max value of _A to be 1 above the currently tested and proved to work over time on Curve.

In Curve the max value of _A = 1e6 - 1 ( _future_A < MAX_A )

MAX_A: constant(uint256) = 10 ** 6

@external
def ramp_A(_future_A: uint256, _future_time: uint256):
    ... skipped code
    assert (_future_A > 0) and (_future_A < MAX_A) <@ max value for _A = 1e6 - 1

However, Thorn allows the admin to set _A to 1e6 via initialize(). (1 above the max value from Curve). Source: StableSwapThreePool.sol and StableSwapTwoPool.sol

uint256 public constant MAX_A = 1e6;

    function initialize(
        ...
    ) external {
        ... skipped code
        require(_A <= MAX_A, "_A exceeds maximum"); <@ max value of _A = 1e6

.

It's crucial to maintain the exact semantics of such important parameters, especially in financial contracts where small differences can have significant impacts. The safer approach is to stick with the stricter condition from the original Curve code.

Attack Scenario

• An admin sets the value of _Ato 1e6 inside initialize() (the validation logic in Thorn contracts allows it. It can be assumed that it's not an admin error, since there is a strict validation inside initialize() for the _A value ranges)
• Since this value was never tested on Curve, the pool's behavior might not be predictable. In case of an issues, the protocol cannot change the value of _A immediately, due to the time checks in StableSwapThreePool::ramp_A() and StableSwapTwoPool::ramp_A()

Attachments

1. Proof of Concept (PoC) File

https://github.com/curvefi/curve-contract/blob/master/contracts/pools/3pool/StableSwap3Pool.vy#L708

https://github.com/hats-finance/Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d/blob/main/contracts/stableSwap/plain-pools/StableSwapTwoPool.sol#L142

https://github.com/hats-finance/Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d/blob/main/contracts/stableSwap/plain-pools/StableSwapThreePool.sol#L141

2. Revised Code File (Optional)

Modify StableSwapTwoPooland StableSwapThreePool validation logic for _A to match the one from Curve

uint256 public constant MAX_A = 1e6;

    function initialize(
        ...
    ) external {
        ... 
        require(_A < MAX_A, "_A exceeds maximum");// < instead of <=
        ...
}

[omega-audits]

Invalid since the admin has to knowingly set it to this value on initialize

[dinkras]

I agree it will be considered an admin error if the Thorn docs have those values documented for the admins, however they do not. In this case the other source of truth for the admin is the code itself, which clearly allows _A value, not supported on Curve.

require(_A <= MAX_A, "_A exceeds maximum") // 1e6 _A value allowed, Curve supports max 1e6 - 1