# OffchainExchange::swapAmm() does not validate that txn.priceX18 > 0, allowing donation attacks Crit/High · Three Sigma · Hybrid orderbook DEX · 3rd April 2024 Finding 3S-Vertex-H02 of the Vertex security review. - Report: /reports/vertex - Source: https://github.com/0xsimao/audits/blob/main/Three%20Sigma/2024-04-03-vertex.pdf --- ### Description OffchainExchange::swapAmm() calculates the quote amount as -txn.amount.mul(txn.priceX18). Thus, if the price is negative, base and quote amount will have the same sign. If it's negative, the transaction will revert as it would lead to a smaller k in the amm. However, if it's positive, it will provide both base and quote to the amm and the user will take a big loss. Using a txn.price < 0, it can be used to perform donation attacks, which could be dangerous, as it maintains the base/quote ratio but inflates lp value. An example of an attack is the following: Price = 10, attacker mints 11 lp using 1 base and 10 quote. Attacker calls OffchainExchange::swapAmm() with txn.amount = 1e21 and txn.price = -10. This changes the amm state to 11 lp, approx 1e21 base and 1e22 quote. Some user adds lp with 1e20 base and 1e21 quote (note that the price is maintained) toMint = 1e20/1e21*11, which rounds down to 0, stealing the funds. Additionally, it is [recommended](https://github.com/vertex-protocol/vertex-contracts-3sigma-audit/blob/4286c37cf4ee59d7e3231ac4683859f54d0d8e96/contracts/ClearinghouseLiq.sol#L708) to enforce that swaps follow the amm curve to prevent price manipulations. ### Recommendation Ensure that the price is positive, as a negative price would be a user mistake or a donation attack. ### Status Addressed in [#38bbe76](https://github.com/vertex-protocol/vertex-contracts-3sigma-audit/commit/38bbe768d64139b9de91a065f4bdd1cb1e212198). Disclosed by 0xSimao (https://0xsimao.com/). --- Related findings: - [SDAIPriceFeed is vulnerable to donation attack via manipulatable vault rate](https://0xsimao.com/findings/evro-finance-ii-sdai-price-donation-manipulatable): Evro Collateral Onboarding - [Curve multi exchange does not validate assetIn and assetOut against route](https://0xsimao.com/findings/singularity-curve-validate-against-route): Singularity - [Distribute is permissionless, allowing malicious users to specify 0 slippage and sandwich the swap](https://0xsimao.com/findings/trestle-permissionless-specify-slippage-sandwich): Trestle wTIA Bridge - [`LenderCommitmentGroup_Smart` picks the wrong Uniswap price, allowing borrowing at a discount by swapping before withdrawing](https://0xsimao.com/findings/teller-finance-picks-uniswap-price-allowing): Teller Finance