search

code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

Tokens can be swapped via unwhitelisted pools #636

Open c4-bot-4 opened 9 months ago

c4-bot-4 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L366 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L382 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L395

Vulnerability details

Impact

Tokens can be swapped via unwhitelisted pools, and perform arbitrages, despite this should only be available for whitelisted pools.

Swaps generate stats used for reward calculations that will only be distributed for whitelisted pools.

Vulnerability Details

The Pool contract has several swap methods, that use protocol pools, and perform arbitrages on them. This functionality should only be available for whitelisted pools.

The swap methods affected are:

None of these functions verify that the corresponding pool is actually whitelisted.

Proof of Concept

  1. Copy this test to src/pools/tests/Pools2.t.sol
  2. Run COVERAGE="no" NETWORK="sep" forge test -vv --rpc-url https://sepolia.gateway.tenderly.co --mt "testSwapViaUnwhitelistedPool"
function testSwapViaUnwhitelistedPool() public {
    // Verify that the pool was already whitelisted
    bytes32 poolID = PoolUtils._poolID(tokens[5], tokens[7]);
    assertTrue(poolsConfig.isWhitelisted(poolID));

    // Unwhitelist the pool
    vm.prank(address(dao));
    poolsConfig.unwhitelistPool(pools, tokens[5], tokens[7]);

    // Verify that the pool was UNWHITELISTED
    assertEq(poolsConfig.isWhitelisted(poolID), false);

    // Perform a swap via the unwhitelisted pool (tokens[5], tokens[7])
    vm.startPrank(address(DEPLOYER));
    uint256 startBalanceTokenOut = tokens[5].balanceOf(address(DEPLOYER));
    uint256 swapAmountIn = 1 ether;
    uint256 swapAmountOut = pools.depositSwapWithdraw(tokens[5], tokens[7], swapAmountIn, 0, block.timestamp + 1 minutes);

    // Verify that the swap was executed successfully via the unwhitelisted pool
    uint256 endBalanceTokenOut = tokens[7].balanceOf(address(DEPLOYER));
    assertEq(endBalanceTokenOut - startBalanceTokenOut, swapAmountOut, "Incorrect swap amount transfered to the sender");
}

Recommended Mitigation Steps

Check that the pool for the swapped tokens is whitelisted with this check:

require(poolsConfig.isWhitelisted(poolID), "Invalid pool");

It could be added in the _adjustReservesForSwap() function, which is used by all swap functions right after the poolID is calculated

Assessed type

Other

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #965

c4-judge commented 8 months ago

Picodes changed the severity to QA (Quality Assurance)