Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inappropriate slippage parameter settings. #304

Closed
c4-bot-10 opened this issue Jan 27, 2024 · 2 comments
Closed

Inappropriate slippage parameter settings. #304

c4-bot-10 opened this issue Jan 27, 2024 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-805 unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-10
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L62
https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L72

Vulnerability details

Impact

Using inappropriate slippage parameters when calculating zapAmount.
The provided minAmountOut is set to 0.
the slippage control the user can receive the least optimal amount of the token they want to trade.
Users may incur losses of assets when adding liquidity.

// Deposit an arbitrary amount of one or both tokens into the pool and receive liquidity corresponding the the value of both of them.
	// As the ratio of tokens added to the pool has to be the same as the existing ratio of reserves, some of the excess token will be swapped to the other.
	// Due to precision reduction during zapping calculation, the minimum possible reserves and quantity possible to zap is .000101,
	function _dualZapInLiquidity(IERC20 tokenA, IERC20 tokenB, uint256 zapAmountA, uint256 zapAmountB ) internal returns (uint256 amountForLiquidityA, uint256 amountForLiquidityB  )
		{
		(uint256 reserveA, uint256 reserveB) = pools.getPoolReserves(tokenA, tokenB);
		(uint256 swapAmountA, uint256 swapAmountB ) = PoolMath._determineZapSwapAmount( reserveA, reserveB, zapAmountA, zapAmountB );

		// tokenA is in excess so swap some of it to tokenB?
		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 );
			}

		return (zapAmountA, zapAmountB);
		}

And the deadline is set to block.timestamp.
code-423n4/2022-11-paraspace-findings#429 as a validator can hold the transaction and the block it is eventually put into will be block.timestamp, so this offers no protection.
This could result in users executing transactions at unfavorable prices.

Proof of Concept

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L62
https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L72

Tools Used

Recommended Mitigation Steps

Implement proper slippage control.

Assessed type

MEV

@c4-bot-10 c4-bot-10 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jan 27, 2024
c4-bot-4 added a commit that referenced this issue Jan 27, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Feb 1, 2024

Picodes marked the issue as duplicate of #805

@c4-judge c4-judge closed this as completed Feb 1, 2024
@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Feb 18, 2024
@c4-judge
Copy link
Contributor

Picodes marked the issue as unsatisfactory:
Invalid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-805 unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants