In the UpliftOnlyExample
contract, protocol fees that are collected during withdrawals may become permanently locked in the contract due to using maxAmountsIn
instead of exact amounts in when minting BPT's for the admin. This leads to a loss of yield for the protocol as portions of collected fees become inaccessible.
The issue occurs in the fee collection logic where maxAmountsIn
is used with localData.accruedQuantAMMFees
:
When BPT's are minted for the admin during withdrawals, the full accrued fee amount is specified as a maximum input amount rather than specifying an exact input amount. Since the operation uses AddLiquidityKind.PROPORTIONAL
which uses exact output amounts, this means an amount UP to the accruedQuantAMMFees
will be used, but not necessarily the full amount. The net effect is:
The full fee amount is deducted from users during withdrawals
Only a portion of those fees may actually be transferred to the protocol
The unused portion remains locked in the contract with no mechanism to recover it
This creates a discrepancy where the users amount is reduced by accruedQuantAMMFees
but the protocol mints BTPS's where the amounts in are <= accruedQuantAMMFees
. In every case that the amount in is less than accruedQuantAMMFees
the protocol will be left with a locked amount of fees that cannot be accessed.
Additionally:
There is another issue with the current logic. The fix for the two is the same which is why I am mentioning both in this report.
Due to the way maxAmountsIn
is calculated there will be times it will be less then amountInRaw
. When this happens the call will revert and throw the AmountInAboveMax
error. This is detrimental to the protocol because this means that the users wont be able to remove liquidity if the amount they are removing is impacted by the rounding nature of the calculations that take place in this flow. The liquidity they wont be able to remove will be locked not because the size of there position is malicious but because the admin fee calculations are wrong and create impossible to satisfy slippage parameters.
In this PoC you will see that removing liquidity at times will revert when an admin fee exist. It seems this was not caught since admin fees were set to 0 in the test suite. But once set to even 5 basis points the issue presents itself.
By running this test in UpliftExample.t.sol
you will see a revert with the message:
It is important to note that I ran this test with some of the other fixes from my other issues implemented and the same revert exist just with more sensible numbers.
All in all these are two separate issues both of which with high severity however because it can be resolved with one fix as mentioned below I have this as a single high severity issue.
Loss of yield and Locked funds - Protocol fees that are collected but not properly transferred become permanently locked in the contract, depriving the protocol of revenue it should have received.
Manual Review
Modify the logic to use exact input amounts instead of exact output amounts to ensure all collected fees are properly transferred to the protocol:
This ensures that:
The exact fee amount collected from users is transferred to the protocol
No fees remain trapped in the contract
The protocol receives all revenue it is entitled to
Positions wont become stuck due to bad slippage calculations.
Likelihood: High, multiple rounding down and little value can trigger the bug Impact: High, DoS the removal.
Likelihood: Medium/High, everytime maxAmountsIn is bigger than need for the exact amount of BPT. Impact: High, fees stuck in the contract.
Likelihood: High, multiple rounding down and little value can trigger the bug Impact: High, DoS the removal.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.