# Turbos audit report (version 2.0)
## [H-01] pool.move: Incorrect `fee_growth_above` condition
### Code Snippet
```rust
fun next_fee_growth_inside<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
tick_lower_index: I32,
tick_upper_index: I32,
tick_current_index: I32,
_ctx: &mut TxContext,
): (u128, u128) {
let tick_lower = get_tick(pool, tick_lower_index);
let tick_upper = get_tick(pool, tick_upper_index);
// calculate fee growth below
let fee_growth_below_a;
let fee_growth_below_b;
if (!tick_lower.initialized) {
fee_growth_below_a = pool.fee_growth_global_a;
fee_growth_below_b = pool.fee_growth_global_b;
} else if (i32::gte(tick_current_index, tick_lower_index)) {
fee_growth_below_a = tick_lower.fee_growth_outside_a;
fee_growth_below_b = tick_lower.fee_growth_outside_b;
} else {
fee_growth_below_a = math_u128::wrapping_sub(pool.fee_growth_global_a, tick_lower.fee_growth_outside_a);
fee_growth_below_b = math_u128::wrapping_sub(pool.fee_growth_global_b, tick_lower.fee_growth_outside_b);
};
// calculate fee growth above
let fee_growth_above_a;
let fee_growth_above_b;
// @audit-issue Should check tick_upper.initialized instead
if (!tick_lower.initialized) {
fee_growth_above_a = 0;
fee_growth_above_b = 0;
} else if (i32::lt(tick_current_index, tick_upper_index)) {
fee_growth_above_a = tick_upper.fee_growth_outside_a;
fee_growth_above_b = tick_upper.fee_growth_outside_b;
} else {
fee_growth_above_a = math_u128::wrapping_sub(pool.fee_growth_global_a, tick_upper.fee_growth_outside_a);
fee_growth_above_b = math_u128::wrapping_sub(pool.fee_growth_global_b, tick_upper.fee_growth_outside_b);
};
let fee_growth_inside_a = math_u128::wrapping_sub(
math_u128::wrapping_sub(pool.fee_growth_global_a, fee_growth_below_a), fee_growth_above_a
);
let fee_growth_inside_b = math_u128::wrapping_sub(
math_u128::wrapping_sub(pool.fee_growth_global_b, fee_growth_below_b), fee_growth_above_b
);
(fee_growth_inside_a, fee_growth_inside_b)
}
```
### Bug Description
The condition if (`!tick_lower.initialized`) is used to determine if the fee growth above should be set to zero. However, this condition should be checking `tick_upper.initialized` instead. The current logic incorrectly assumes that the lower tick's initialization status is relevant for calculating the fee growth above the current tick range.
### Recommendation
Fix the condition to check the upper tick's initialization status:
```rust
// calculate fee growth above
let fee_growth_above_a;
let fee_growth_above_b;
if (!tick_upper.initialized) { // <-- FIXED
fee_growth_above_a = 0;
fee_growth_above_b = 0;
} else if (i32::lt(tick_current_index, tick_upper_index)) {
fee_growth_above_a = tick_upper.fee_growth_outside_a;
fee_growth_above_b = tick_upper.fee_growth_outside_b;
} else {
fee_growth_above_a = math_u128::wrapping_sub(pool.fee_growth_global_a, tick_upper.fee_growth_outside_a);
fee_growth_above_b = math_u128::wrapping_sub(pool.fee_growth_global_b, tick_upper.fee_growth_outside_b);
};
```
### [H-02] pool.move: `check_ticks()` function lacks tick spacing validation
### Code snippet
```rust
fun check_ticks(
tick_lower_index: I32,
tick_upper_index: I32
) {
assert!(i32::lt(tick_lower_index, tick_upper_index), EInvildTick);
assert!(i32::gte(tick_lower_index, i32::neg_from(MAX_TICK_INDEX)), EInvildTick);
assert!(i32::lte(tick_upper_index, i32::from(MAX_TICK_INDEX)), EInvildTick);
}
```
### Bug description
The `check_ticks()` function only validates that:
- Lower tick is less than upper tick
- Ticks are within `MAX_TICK_INDEX` bounds
However, it does not validate that the ticks are properly spaced according to the pool's tick_spacing parameter. While `flip_tick()` does this validation, the `mint()` function can create positions with ticks that don't align with the pool's tick spacing.
### PoC
```rust
// Assume pool.tick_spacing = 60
// Attacker can create position with invalid ticks:
let tick_lower = 10; // Should be multiple of 60
let tick_upper = 100; // Should be multiple of 60
pool::mint(
pool,
owner,
i32::from(tick_lower), // Will pass check_ticks() but is invalid spacing
i32::from(tick_upper), // Will pass check_ticks() but is invalid spacing
liquidity_delta,
clock,
ctx
);
```
### Recommendation
Add tick spacing validation in `check_ticks()`:
```rust
fun check_ticks(
tick_lower_index: I32,
tick_upper_index: I32,
tick_spacing: u32
) {
assert!(i32::lt(tick_lower_index, tick_upper_index), EInvildTick);
assert!(i32::gte(tick_lower_index, i32::neg_from(MAX_TICK_INDEX)), EInvildTick);
assert!(i32::lte(tick_upper_index, i32::from(MAX_TICK_INDEX)), EInvildTick);
// Add tick spacing validation
assert!(i32::eq(i32::mod_euclidean(tick_lower_index, tick_spacing), i32::zero()), EInvildTickIndex);
assert!(i32::eq(i32::mod_euclidean(tick_upper_index, tick_spacing), i32::zero()), EInvildTickIndex);
}
```
And update the call in `modify_position()`:
```rust
fun modify_position<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
owner: address,
tick_lower_index: I32,
tick_upper_index: I32,
liquidity_delta: I128,
clock: &Clock,
ctx: &mut TxContext,
) {
check_ticks(tick_lower_index, tick_upper_index, pool.tick_spacing);
// ... rest of function
}
```
## [H-03] position_manager.move: Incorrect slippage check condition in `decrease_liquidity_with_return_()`
### Code snippet
```rust
public fun decrease_liquidity_with_return_<CoinTypeA, CoinTypeB, FeeType>(
...
): (Coin<CoinTypeA>, Coin<CoinTypeB>) {
...
// @audit-issue amount_b_min >= amount_b_min is always true
assert!(amount_a >= amount_a_min && amount_b_min >= amount_b_min, EPriceSlippageCheck);
...
}
```
### Bug description
A critical vulnerability exists in the decrease_liquidity_with_return_ function where the price slippage check is incorrectly implemented, potentially allowing transactions to proceed even when the output amount is below the minimum specified threshold.
The bug is in the following line:
```rust
assert!(amount_a >= amount_a_min && amount_b_min >= amount_b_min, EPriceSlippageCheck);
```
The issue is that instead of comparing `amount_b` with `amount_b_min`, the code compares `amount_b_min` with itself (`amount_b_min >= amount_b_min`). This comparison will always evaluate to true, effectively nullifying the slippage protection for token B.
### PoC
Consider this scenario:
1. User calls `decrease_liquidity_with_return_()` with:
- `amount_b_min` = 100
- Actual `amount_b` received = 50
2. The check `amount_b_min >= amount_b_min` will always return true (100 >= 100)
3. Transaction proceeds despite user receiving 50% less of token B than their minimum specified amount
### Recommendation
Fix the assertion to properly compare the actual amount received with the minimum amount:
```rust
assert!(amount_a >= amount_a_min && amount_b >= amount_b_min, EPriceSlippageCheck);
```
## [H-04] Missing deadline check in flash swap
### Code snippet
```rust
public fun flash_swap<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
recipient: address,
a_to_b: bool,
amount_specified: u128,
amount_specified_is_input: bool,
sqrt_price_limit: u128,
clock: &Clock,
versioned: &Versioned,
ctx: &mut TxContext,
): (Coin<CoinTypeA>, Coin<CoinTypeB>, FlashSwapReceipt<CoinTypeA, CoinTypeB>) {
check_version(versioned);
assert!(pool.unlocked, EPoolLocked);
// No deadline check here
...
}
public fun repay_flash_swap<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
coin_a: Coin<CoinTypeA>,
coin_b: Coin<CoinTypeB>,
receipt: FlashSwapReceipt<CoinTypeA, CoinTypeB>,
versioned: &Versioned
) {
// No deadline check here either
...
}
```
### Bug description
While regular swaps through the router have deadline checks, flash swaps can be executed at any time with no deadline protection. This means:
1. Flash swap transactions can be held by validators indefinitely
2. The repayment can also be delayed indefinitely
3. No protection against sandwich attacks or unfavorable price movements
### Recommendation
Add deadline parameter to flash swap operations:
```rust
public fun flash_swap<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
recipient: address,
a_to_b: bool,
amount_specified: u128,
amount_specified_is_input: bool,
sqrt_price_limit: u128,
deadline: u64, // Add deadline parameter
clock: &Clock,
versioned: &Versioned,
ctx: &mut TxContext,
): (Coin<CoinTypeA>, Coin<CoinTypeB>, FlashSwapReceipt<CoinTypeA, CoinTypeB>) {
check_version(versioned);
assert!(pool.unlocked, EPoolLocked);
assert!(clock::timestamp_ms(clock) <= deadline, ETransactionToOld);
...
}
// Also modify FlashSwapReceipt to include deadline
struct FlashSwapReceipt<phantom CoinTypeA, phantom CoinTypeB> {
pool_id: ID,
a_to_b: bool,
pay_amount: u64,
deadline: u64, // Add deadline
}
public fun repay_flash_swap<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
coin_a: Coin<CoinTypeA>,
coin_b: Coin<CoinTypeB>,
receipt: FlashSwapReceipt<CoinTypeA, CoinTypeB>,
clock: &Clock, // Add clock parameter
versioned: &Versioned
) {
assert!(clock::timestamp_ms(clock) <= receipt.deadline, ETransactionToOld);
...
}
```
## [H-05] position_manager.move: `burn_position_nft_with_return_()` lacks a few important checks before buring
### Code snippet
```rust
public fun burn_position_nft_with_return_<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
positions: &mut Positions,
nft: TurbosPositionNFT,
versioned: &Versioned,
ctx: &mut TxContext
): TurbosPositionBurnNFT {
pool::check_version(versioned);
let pool_id = position_nft::pool_id(&nft);
assert!(object::id(pool) == position_nft::pool_id(&nft), EInvalidPool);
let nft_address = object::id_address(&nft);
let position_inner = dof::borrow_mut<address, Position>(&mut positions.id, nft_address);
let tick_spacing = pool::get_pool_tick_spacing(pool);
let mod = i32::from(TICK_SIZE % tick_spacing);
//check full range
assert!(i32::eq(i32::sub(position_inner.tick_lower_index, mod), i32::neg_from(TICK_SIZE)), EInvalidBurnTickRange);
assert!(i32::eq(i32::add(position_inner.tick_upper_index, mod), i32::from(TICK_SIZE)), EInvalidBurnTickRange);
let position_id = position_nft::position_id(&nft);
let burn_nft = TurbosPositionBurnNFT{
id: object::new(ctx),
name: string::utf8(b"Proof of Turbos Position Burn"),
description: string::utf8(b"Proof of Turbos Position Burn"),
img_url: url::new_unsafe(string::to_ascii(string::utf8(b"https://app.turbos.finance/icon/turbos-position-burn-nft.png"))),
position_nft: nft,
position_id: position_id,
pool_id: pool_id,
coin_type_a: type_name::get<CoinTypeA>(),
coin_type_b: type_name::get<CoinTypeB>(),
fee_type: type_name::get<FeeType>(),
};
event::emit(BurnPositionEvent {
nft_address: nft_address,
position_id: position_id,
pool_id: pool_id,
burn_nft_address: object::id_address(&burn_nft),
});
burn_nft
}
```
### Bug description
The `burn_position_nft_with_return_()` function in position_manager.move converts a position NFT to a burn NFT without verifying that all assets have been withdrawn from the position. This could result in permanent loss of:
- Active liquidity in the pool
- Uncollected trading fees
- Uncollected rewards
### PoC
```rust
public fun burn_position_nft_with_return_<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
positions: &mut Positions,
nft: TurbosPositionNFT,
versioned: &Versioned,
ctx: &mut TxContext
) -> TurbosPositionBurnNFT {
// Only checks pool version and tick range
// Does NOT check for remaining assets
pool::check_version(versioned);
let nft_address = object::id_address(&nft);
let position_inner = dof::borrow_mut<address, Position>(&mut positions.id, nft_address);
// Position could still have:
// - position_inner.liquidity > 0
// - position_inner.tokens_owed_a > 0
// - position_inner.tokens_owed_b > 0
// - reward_info.amount_owed > 0
// Creates burn NFT without ensuring position is cleared
let burn_nft = TurbosPositionBurnNFT{ ... };
burn_nft
}
```
### Recommendation
Add checks to ensure the position is fully cleared before allowing the burn:
```rust
public fun burn_position_nft_with_return_<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
positions: &mut Positions,
nft: TurbosPositionNFT,
versioned: &Versioned,
ctx: &mut TxContext
) -> TurbosPositionBurnNFT {
pool::check_version(versioned);
let nft_address = object::id_address(&nft);
let position = dof::borrow_mut<address, Position>(&mut positions.id, nft_address);
// Check no liquidity
assert!(position.liquidity == 0, EPositionNotCleared);
// Check no uncollected fees
assert!(position.tokens_owed_a == 0 && position.tokens_owed_b == 0, EPositionNotCleared);
// Check no uncollected rewards
let i = 0;
let len = vector::length(&position.reward_infos);
while (i < len) {
let reward_info = vector::borrow(&position.reward_infos, i);
assert!(reward_info.amount_owed == 0, EPositionNotCleared);
i = i + 1;
};
// Rest of the function...
}
```
## [H-06] pool.move: Hardcoded reward index in modify_tick_reward() and modify_tick()
### Code snippet
```rust
public(friend) fun modify_tick_reward<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
tick_lower_index: I32,
tick_upper_index: I32,
) {
// Always uses index 0
let reawrd_info = vector::borrow(reward_infos, 0);
modify_tick_reward_outside(pool, tick_lower_index, 0, reawrd_info.growth_global);
// ...
}
```
```rust
public(friend) fun modify_tick<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
tick_index: I32,
) {
if (i32::lte(tick_index, pool.tick_current_index)) {
let reward_infos = &pool.reward_infos;
let reawrd_info = vector::borrow(reward_infos, 0); // Always uses index 0
modify_tick_reward_outside(pool, tick_index, 0, reawrd_info.growth_global);
} else {
modify_tick_reward_outside(pool, tick_index, 0, 0); // Always uses index 0
};
}
```
### Bug description
`modify_tick_reward()` and `modify_tick()` in the pool contract incorrectly handle reward indices by hardcoding them to 0, leading to broken reward accounting for all reward tokens beyond the first one.
### PoC
- Set up a pool with 2 reward tokens
- Add liquidity and generate rewards
- Call `modify_tick_reward` or `modify_tick`
- Observe that only the first reward token's growth values are updated
- Second reward token's accounting remains unchanged
### Recommendation
Add another input parameter `reward_index` to specify which reward token to modify.
## [H-07] position_manager.move: Missing tick range validation in multiple functions
### Code snippet
1. `mint_with_return_()`
2. `increase_liquidity_with_return_()`
3. `decrease_liquidity_with_return_()`
### Bug description
Several functions in the position_manager.move contract lack proper validation for `tick_lower_index` and `tick_upper_index`. Specifically, they don't verify that:
1. `tick_lower_index` is less than `tick_upper_index`
2. The ticks are within the valid range (-443636 to 443636)
3. The ticks are properly spaced according to the pool's tick spacing
### Recommendation
Add proper tick range validations in all position management functions. Example fix for `mint_with_return_()`:
```rust
public fun mint_with_return_<CoinTypeA, CoinTypeB, FeeType>(
// ... parameters ...
) {
let tick_lower_index_i32 = i32::from_u32_neg(tick_lower_index, tick_lower_index_is_neg);
let tick_upper_index_i32 = i32::from_u32_neg(tick_upper_index, tick_upper_index_is_neg);
// Add validations
assert!(i32::lt(tick_lower_index_i32, tick_upper_index_i32), EInvalidTickRange);
assert!(i32::ge(tick_lower_index_i32, i32::neg_from(TICK_SIZE)), EInvalidTickRange);
assert!(i32::le(tick_upper_index_i32, i32::from(TICK_SIZE)), EInvalidTickRange);
let tick_spacing = pool::get_pool_tick_spacing(pool);
assert!(i32::to_u32(tick_lower_index_i32) % tick_spacing == 0, EInvalidTickSpacing);
assert!(i32::to_u32(tick_upper_index_i32) % tick_spacing == 0, EInvalidTickSpacing);
// ... rest of the code ...
}
```
Similar validations should be added to `increase_liquidity_with_return_()` and `decrease_liquidity_with_return_()` functions.
## [L-01] Deprecated functions should be removed
There are a few deprecated in the codebase, for example:
```rust
public(friend) fun collect_reward<CoinTypeA, CoinTypeB, FeeType, RewardCoin>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
vault: &mut PoolRewardVault<RewardCoin>,
recipient: address,
tick_lower_index: I32,
tick_upper_index: I32,
amount: u64,
reward_index: u64,
ctx: &mut TxContext
): u64 {
abort(0)
}
```
If they are not intended to be used any more in future it is better to remove them.
## [L-02] All users share the `same nft_img_url`
The position nft url is set in `position_manager::init_()`:
```rust
fun init_(ctx: &mut TxContext) {
transfer::share_object(Positions {
id: object::new(ctx),
nft_minted: 0,
user_position: table::new(ctx),
nft_name: string::utf8(b"Turbos Position's NFT"),
nft_description: string::utf8(b"An NFT created by Turbos CLMM"),
nft_img_url: string::utf8(b"https://ipfs.io/ipfs/QmTxRsWbrLG6mkjg375wW77Lfzm38qsUQjRBj3b2K3t8q1?filename=Turbos_nft.png"),
});
}
```
All users who mint position nft will share this one single `nft_img_url`. There are a few potential issues:
1. Single Point of Failure:
- If the IPFS link becomes unavailable, all NFTs would lose their images simultaneously
- If the image is hosted on a centralized server, server downtime would affect all NFTs
2. Centralization Risk:
- The protocol owner (pool factory) could change all NFT images at once through `update_nft_img_url()`
- This gives too much control to the protocol owner and goes against decentralization principles
3. Lack of Uniqueness:
- All NFTs having the same image reduces their uniqueness and value proposition
Makes it harder to visually distinguish between different positions
4. Resource Efficiency:
- Multiple requests to the same URL could lead to rate limiting or excessive load on the hosting service
## [L-03] pool.move: Zero Liquidity Swaps Allowed
```rust
public(friend) fun compute_swap_result<CoinTypeA, CoinTypeB, FeeType>(
// ...
) {
//assert!(state.liquidity > 0, EInsufficientLiquidity);
// ... rest of the function
}
```
The code has commented out a critical check for zero liquidity. This means swaps can occur even when there's no liquidity in the pool, which could lead to:
- Division by zero errors
- Incorrect price calculations
- Potential manipulation of pool state
## [L-04] pool.move: Uncontrolled Fee Protocol Initialization
The `fee_protocol` parameter can be set to any arbitrary value during pool deployment without proper validation. This allows potential manipulation of protocol fee percentages, which could lead to unexpected economic impacts on the pool's fee collection mechanism.
Exploit Method:
1. When calling `deploy_pool`, a malicious deployer can set an extremely high or manipulated `fee_protocol` value.
2. Since there's no upper bound or validation check on the `fee_protocol` parameter, this could lead to:
- Disproportionate fee extraction
- Potential economic disruption of the pool's fee distribution model
Practical Exploit Scenario:
```rust
// A malicious deployer could deploy a pool with an extreme fee protocol value
let malicious_pool = deploy_pool<CoinA, CoinB, FeeType>(
500, // standard fee
1, // tick spacing
sqrt_price, // sqrt price
99999, // extremely high fee protocol percentage
&clock,
ctx
);
```
The key issue is the lack of a constraint on `fee_protocol`, which should typically have a predefined, limited range (e.g., 0-100%) to prevent economic manipulation.
**Recommendation:**
```rust
// Add validation during pool deployment
if fee_protocol > 100 {
abort!(INVALID_FEE_PROTOCOL);
}
```
### [L-05] pool.move: Missing fee tier and tick spacing validation during deployment
```rust
public(friend) fun deploy_pool<CoinTypeA, CoinTypeB, FeeType>(
fee: u32,
tick_spacing: u32,
sqrt_price: u128,
fee_protocol: u32,
clock: &Clock,
ctx: &mut TxContext
) : Pool<CoinTypeA, CoinTypeB, FeeType> {
// No validation for fee tier and tick spacing
```
The function allows completely unrestricted setting of fee and tick_spacing without any validation against the standard Uniswap V3 protocol specifications. This can lead to creating pools with invalid or extreme configurations that deviate from the expected protocol design:
- No check for tick_spacing being non-zero
- No validation that tick_spacing is a power of 2
- Could lead to division by zero errors or incorrect tick calculations
A malicious deployer can manipulate the pool creation by passing arbitrary values for:
- `fee`: Uniswap V3 typically uses predefined fee tiers (0.05%, 0.3%, 1%)
- `tick_spacing`: Should align with specific fee tiers
For example:
```rust
// Create a pool with an extreme, non-standard configuration
let malicious_pool = deploy_pool<SUI, USDC, FeeType>(
10000, // Extremely high fee (100%)
1, // Minimal tick spacing
sqrt_price,
fee_protocol,
clock,
ctx
)
```
**Recommendation:**
Implement strict validation for `fee` and `tick_spacing` to match Uniswap V3 standard configurations:
- Restrict `fee` to predefined tiers (0.05%, 0.3%, 1%)
- Enforce tick spacing based on fee tier
- Add explicit validation checks before pool creation
## [Informational-01] Fix typos in the codebase
There are some cases where variable names are misspelled, for example `reawrd_info` should be `reward_info`:
```rust
public(friend) fun modify_tick<CoinTypeA, CoinTypeB, FeeType>(
pool: &mut Pool<CoinTypeA, CoinTypeB, FeeType>,
tick_index: I32,
) {
if (i32::lte(tick_index, pool.tick_current_index)) {
let reward_infos = &pool.reward_infos;
let reawrd_info = vector::borrow(reward_infos, 0);
modify_tick_reward_outside(pool, tick_index, 0, reawrd_info.growth_global);
} else {
modify_tick_reward_outside(pool, tick_index, 0, 0);
};
}
```
Sign in with Wallet
Connect another wallet