Changing A could allow an attacker to withdraw huge token balances when the change happens

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x79687f12e95bd5f9d790075c3250ef8857ee1403be91bfd137c9caedc4edcbe2 Severity: medium

Description: Description\ StableSwapThreePool and StableSwapTwoPool pools have an amplicifation coefficient(A) that can be adjusted depending on the peg of the stablecoins in the pool and the needed liquidity concentration. This can happen after a pool is deployed when there is not sufficient concentration or when the stablecoin peg has been changed to allow better trading.

An admin can schedule changing of A with the ramp_A function and can set a valid and reasonable increase or decrease of A, which could expose the pool to a loss.

There is a detailed report about a vulnerabilty in Curve which can be found https://medium.com/@peter_4205/curve-vulnerability-report-a1d7630140ec. To mitigate the issue, in Curve implementation, the amplicifation coefficient(A) is changed with some limitation shown Stableswap.vy#L1061-L1080,

1060 @external
1061 def ramp_A(_future_A: uint256, _future_time: uint256):
...

>> In Curve's version, the function uses _future_A * A_PRECISION to check

1066     _initial_A: uint256 = self._A()
1067     _future_A_p: uint256 = _future_A * A_PRECISION
1068 
1069     assert _future_A > 0 and _future_A < MAX_A
1070     if _future_A_p < _initial_A:
1071         assert _future_A_p * MAX_A_CHANGE >= _initial_A
1072     else:
1073         assert _future_A_p <= _initial_A * MAX_A_CHANGE
1074 
..
1080     log RampA(_initial_A, _future_A_p, block.timestamp, _future_time)
1081 

And take

824     function ramp_A(uint256 _future_A, uint256 _future_time) external onlyOwner {
...

>>> the check is diffreent from Curve's version

828         uint256 _initial_A = get_A();
829         require(_future_A > 0 && _future_A < MAX_A, "_future_A must be between 0 and MAX_A");
830         require(
831             (_future_A >= _initial_A && _future_A <= _initial_A * MAX_A_CHANGE) ||
832                 (_future_A < _initial_A && _future_A * MAX_A_CHANGE >= _initial_A),
833             "Illegal parameter _future_A"
834         );
...
841     }

Attack Scenario\ Please check https://medium.com/@peter_4205/curve-vulnerability-report-a1d7630140ec and https://github.com/hats-finance/Common--Stableswap-0xd4d9a2772202ce33b24901d3fc94e95a84b37430/issues/39 for more details

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

[Ghoulouis]

I just looked through our code, it looks like ramp_A is implemented.