Platform: Code4rena
Start Date: 26/09/2022
Pot Size: $50,000 USDC
Total HM: 13
Participants: 113
Period: 5 days
Judge: 0xean
Total Solo HM: 6
Id: 166
League: ETH
Rank: 9/113
Findings: 2
Award: $910.46
🌟 Selected for report: 0
🚀 Solo Findings: 0
878.9235 USDC - $878.92
AlgebraPool mint function has the ability to return excess funds to the sender. But there are two cases where the user can lose all their supplied tokens.
If the user provides/transfers more tokens than needed inside algebraMintCallback(uint256 amount0Owed, uint256 amount1Owed, bytes calldata data) callback function it will lose all tokens for the following two cases :
The lost tokens are not accounted for any user positions or balance.
The case when this happen is specified by the AlgebraPool::_getAmountsForLiquidity function :
Next example of a user losing all token1 tokens. As previously explaind the same can happen to token0 tokens.
In the case amount1Owed is equal to zero when we get called in callback:
// inside the sending contract function algebraMintCallback( uint256 amount0Owed, uint256 amount1Owed, bytes calldata data ) external { if (amount0Owed > 0) { TestERC20(pool.token0()).transfer(address(pool), amount0Owed + 11); // send also this token we should get the surplus returned to the sender TestERC20(pool.token1()).transfer(address(pool), amount1Owed + 3000); } }
When the transaction finishes the pool has correctly returned 11 token0 but no token1 has been returned. So the user lose all 3000 token1.
The 3000 token are later accounted for liquidity by the protocol with the call balanceToken1() in this case.. or balanceToken0 in the other case. But for this user the minting has gone pretty wrong.
Inside AlgebraPool::mint function instead of:
{ if (amount0 > 0) receivedAmount0 = balanceToken0(); if (amount1 > 0) receivedAmount1 = balanceToken1(); IAlgebraMintCallback(msg.sender).algebraMintCallback(amount0, amount1, data); if (amount0 > 0) require((receivedAmount0 = balanceToken0() - receivedAmount0) > 0, 'IIAM'); if (amount1 > 0) require((receivedAmount1 = balanceToken1() - receivedAmount1) > 0, 'IIAM'); }
try to always calculate receivedAmount{0,1} and then check if we need the amountX to be greater than zero, like this:
{ receivedAmount0 = balanceToken0(); receivedAmount1 = balanceToken1(); IAlgebraMintCallback(msg.sender).algebraMintCallback(amount0, amount1, data); receivedAmount0 = balanceToken0() - receivedAmount0; receivedAmount1 = balanceToken1() - receivedAmount1; if (amount0 > 0) require(receivedAmount0 > 0, 'IIAM'); if (amount1 > 0) require(receivedAmount1 > 0, 'IIAM'); }
Now surplus tokens are correctly returned to the sender.
#0 - vladyan18
2022-10-03T15:40:51Z
Thank you! The tokens will indeed not be returned. However, we need to take into account that the required amount is passed in the callback. So this situation can arise only if the contract called in the callback is implemented incorrectly.
#1 - 0xean
2022-10-06T19:21:43Z
going to lump this is as a duplicate of #255 - same root cause.
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x5rings, 0xNazgul, 0xRoxas, 0xSmartContract, 0xbepresent, 0xmatt, Aeros, Amithuddar, Awesome, Aymen0909, B2, Bnke0x0, ChristianKuri, CodingNameKiki, Deivitto, Diraco, Fitraldys, HardlyCodeMan, JC, Mukund, Noah3o6, Olivierdem, RaymondFam, ReyAdmirado, RockingMiles, Rolezn, Ruhum, Saintcode_, Shinchan, SnowMan, TomJ, Tomio, Tomo, V_B, Waze, __141345__, ajtra, asutorufos, aysha, beardofginger, bobirichman, brgltd, bulej93, c3phas, ch0bu, cryptonue, defsec, delfin454000, dharma09, durianSausage, emrekocak, erictee, fatherOfBlocks, francoHacker, gianganhnguyen, gogo, imare, kaden, karanctf, ladboy233, lukris02, m_Rassska, martin, medikko, mics, natzuu, oyc_109, peiw, rbserver, ret2basic, rotcivegaf, saian, shark, slowmoses, tnevler, trustindistrust, zeesaw, zishansami
31.5429 USDC - $31.54
From :
struct Position { uint128 liquidity; // The amount of liquidity concentrated in the range uint32 lastLiquidityAddTimestamp; // Timestamp of last adding of liquidity uint256 innerFeeGrowth0Token; // The last updated fee growth per unit of liquidity uint256 innerFeeGrowth1Token; uint128 fees0; // The amount of token0 owed to a LP uint128 fees1; // The amount of token1 owed to a LP }
to
struct Position { uint256 innerFeeGrowth0Token; // The last updated fee growth per unit of liquidity uint256 innerFeeGrowth1Token; uint128 liquidity; // The amount of liquidity concentrated in the range uint128 fees0; // The amount of token0 owed to a LP uint128 fees1; // The amount of token1 owed to a LP uint32 lastLiquidityAddTimestamp; // Timestamp of last adding of liquidity }
From:
struct Cumulatives { int56 tickCumulative; uint160 outerSecondPerLiquidity; uint32 outerSecondsSpent; }
to:
struct Cumulatives { uint160 outerSecondPerLiquidity; int56 tickCumulative; uint32 outerSecondsSpent; }
From:
struct SwapCalculationCache { uint256 communityFee; // The community fee of the selling token, uint256 to minimize casts uint128 volumePerLiquidityInBlock; int56 tickCumulative; // The global tickCumulative at the moment uint160 secondsPerLiquidityCumulative; // The global secondPerLiquidity at the moment bool computedLatestTimepoint; // if we have already fetched _tickCumulative_ and _secondPerLiquidity_ from the DataOperator int256 amountRequiredInitial; // The initial value of the exact input\output amount int256 amountCalculated; // The additive amount of total output\input calculated trough the swap uint256 totalFeeGrowth; // The initial totalFeeGrowth + the fee growth during a swap uint256 totalFeeGrowthB; IAlgebraVirtualPool.Status incentiveStatus; // If there is an active incentive at the moment bool exactInput; // Whether the exact input or output is specified uint16 fee; // The current dynamic fee int24 startTick; // The tick at the start of a swap uint16 timepointIndex; // The index of last written timepoint }
to
struct SwapCalculationCache { uint256 communityFee; // The community fee of the selling token, uint256 to minimize casts int256 amountRequiredInitial; // The initial value of the exact input\output amount int256 amountCalculated; // The additive amount of total output\input calculated trough the swap uint256 totalFeeGrowth; // The initial totalFeeGrowth + the fee growth during a swap uint256 totalFeeGrowthB; uint160 secondsPerLiquidityCumulative; // The global secondPerLiquidity at the moment uint128 volumePerLiquidityInBlock; int56 tickCumulative; // The global tickCumulative at the moment int24 startTick; // The tick at the start of a swap uint16 fee; // The current dynamic fee uint16 timepointIndex; // The index of last written timepoint IAlgebraVirtualPool.Status incentiveStatus; // If there is an active incentive at the moment bool computedLatestTimepoint; // if we have already fetched _tickCumulative_ and _secondPerLiquidity_ from the DataOperator bool exactInput; // Whether the exact input or output is specified }
From
struct PriceMovementCache { uint160 stepSqrtPrice; // The Q64.96 sqrt of the price at the start of the step int24 nextTick; // The tick till the current step goes bool initialized; // True if the _nextTick is initialized uint160 nextTickPrice; // The Q64.96 sqrt of the price calculated from the _nextTick uint256 input; // The additive amount of tokens that have been provided uint256 output; // The additive amount of token that have been withdrawn uint256 feeAmount; // The total amount of fee earned within a current step }
to
struct PriceMovementCache { uint256 input; // The additive amount of tokens that have been provided uint256 output; // The additive amount of token that have been withdrawn uint256 feeAmount; // The total amount of fee earned within a current step uint160 stepSqrtPrice; // The Q64.96 sqrt of the price at the start of the step uint160 nextTickPrice; // The Q64.96 sqrt of the price calculated from the _nextTick int24 nextTick; // The tick till the current step goes bool initialized; // True if the _nextTick is initialized }
From
struct Configuration { uint16 alpha1; // max value of the first sigmoid uint16 alpha2; // max value of the second sigmoid uint32 beta1; // shift along the x-axis for the first sigmoid uint32 beta2; // shift along the x-axis for the second sigmoid uint16 gamma1; // horizontal stretch factor for the first sigmoid uint16 gamma2; // horizontal stretch factor for the second sigmoid uint32 volumeBeta; // shift along the x-axis for the outer volume-sigmoid uint16 volumeGamma; // horizontal stretch factor the outer volume-sigmoid uint16 baseFee; // minimum possible fee }
to
struct Configuration { uint32 beta1; // shift along the x-axis for the first sigmoid uint32 beta2; // shift along the x-axis for the second sigmoid uint32 volumeBeta; // shift along the x-axis for the outer volume-sigmoid uint16 alpha1; // max value of the first sigmoid uint16 alpha2; // max value of the second sigmoid uint16 gamma1; // horizontal stretch factor for the first sigmoid uint16 gamma2; // horizontal stretch factor for the second sigmoid uint16 volumeGamma; // horizontal stretch factor the outer volume-sigmoid uint16 baseFee; // minimum possible fee }
From
struct Timepoint { bool initialized; // whether or not the timepoint is initialized uint32 blockTimestamp; // the block timestamp of the timepoint int56 tickCumulative; // the tick accumulator, i.e. tick * time elapsed since the pool was first initialized uint160 secondsPerLiquidityCumulative; // the seconds per liquidity since the pool was first initialized uint88 volatilityCumulative; // the volatility accumulator; overflow after ~34800 years is desired :) int24 averageTick; // average tick at this blockTimestamp uint144 volumePerLiquidityCumulative; // the gmean(volumes)/liquidity accumulator }
to
struct Timepoint { uint160 secondsPerLiquidityCumulative; // the seconds per liquidity since the pool was first initialized uint144 volumePerLiquidityCumulative; // the gmean(volumes)/liquidity accumulator uint88 volatilityCumulative; // the volatility accumulator; overflow after ~34800 years is desired :) int56 tickCumulative; // the tick accumulator, i.e. tick * time elapsed since the pool was first initialized uint32 blockTimestamp; // the block timestamp of the timepoint int24 averageTick; // average tick at this blockTimestamp bool initialized; // whether or not the timepoint is initialized }
From
struct Tick { uint128 liquidityTotal; // the total position liquidity that references this tick int128 liquidityDelta; // amount of net liquidity added (subtracted) when tick is crossed left-right (right-left), // fee growth per unit of liquidity on the _other_ side of this tick (relative to the current tick) // only has relative meaning, not absolute — the value depends on when the tick is initialized uint256 outerFeeGrowth0Token; uint256 outerFeeGrowth1Token; int56 outerTickCumulative; // the cumulative tick value on the other side of the tick uint160 outerSecondsPerLiquidity; // the seconds per unit of liquidity on the _other_ side of current tick, (relative meaning) uint32 outerSecondsSpent; // the seconds spent on the other side of the current tick, only has relative meaning bool initialized; // these 8 bits are set to prevent fresh sstores when crossing newly initialized ticks }
to
struct Tick { // fee growth per unit of liquidity on the _other_ side of this tick (relative to the current tick) // only has relative meaning, not absolute — the value depends on when the tick is initialized uint256 outerFeeGrowth0Token; uint256 outerFeeGrowth1Token; uint160 outerSecondsPerLiquidity; // the seconds per unit of liquidity on the _other_ side of current tick, (relative meaning) uint128 liquidityTotal; // the total position liquidity that references this tick int128 liquidityDelta; // amount of net liquidity added (subtracted) when tick is crossed left-right (right-left), int56 outerTickCumulative; // the cumulative tick value on the other side of the tick uint32 outerSecondsSpent; // the seconds spent on the other side of the current tick, only has relative meaning bool initialized; // these 8 bits are set to prevent fresh sstores when crossing newly initialized ticks }