-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
b386bd1
commit 686e1f8
Showing
63 changed files
with
4,799 additions
and
10 deletions.
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
Glorious White Albatross | ||
|
||
Medium | ||
|
||
# External attacker can cause the bypass position protection checks after order invalidation, making the order status inconsistent | ||
|
||
### Summary | ||
|
||
A missing protection flag reset in the Order invalidation logic will cause an inconsistent state issue for market participants as malicious actors can bypass position protection checks after order invalidation. | ||
|
||
### Root Cause | ||
|
||
In [OrderLib.sol](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/types/Order.sol#L87-L92) the `invalidate()` function zeroes out positions but fails to reset the protection flag, leading to an inconsistent state where an order can be both invalidated and protected simultaneously. | ||
|
||
The relevant code : | ||
https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/types/Order.sol#L87-L92 | ||
|
||
### Internal Pre-conditions | ||
|
||
1. A user needs to have a protected order (typically through liquidation) | ||
|
||
### External Pre-conditions | ||
|
||
No | ||
|
||
### Attack Path | ||
|
||
1. Attacker waits for a protected order to be created (e.g., through liquidation) | ||
2. When the oracle reports an invalid price: The order gets invalidated through `invalidate()` function and `Positions `are zeroed but protection flag remains set | ||
3. The attacker can now bypass certain invariant checks that depend on protection status | ||
4. This allows them to execute operations that should be blocked for protected orders | ||
5. The market contract enters an inconsistent state where protection mechanisms can be circumvented | ||
|
||
### Impact | ||
|
||
Protected orders can be manipulated in ways that violate the intended market safety mechanisms | ||
Market invariants is violated as well. | ||
|
||
### PoC | ||
|
||
```ts | ||
describe('#poc', async () => { | ||
beforeEach(async () => { | ||
await market.connect(owner).updateParameter(marketParameter) | ||
dsu.transferFrom.whenCalledWith(user.address, market.address, COLLATERAL.mul(1e12)).returns(true) | ||
dsu.transferFrom.whenCalledWith(userB.address, market.address, COLLATERAL.mul(1e12)).returns(true) | ||
}) | ||
|
||
it('invalidation vulnerability with protected orders', async () => { | ||
// 1. Setup maker position | ||
await market.connect(userB)['update(address,uint256,uint256,uint256,int256,bool)']( | ||
userB.address, | ||
POSITION, | ||
0, | ||
0, | ||
COLLATERAL, | ||
false | ||
) | ||
|
||
// 2. Create a protected order through liquidation | ||
oracle.at.whenCalledWith(ORACLE_VERSION_2.timestamp).returns([ORACLE_VERSION_2, INITIALIZED_ORACLE_RECEIPT]) | ||
oracle.status.returns([ORACLE_VERSION_2, ORACLE_VERSION_3.timestamp]) | ||
oracle.request.whenCalledWith(user.address).returns() | ||
|
||
await settle(market, userB) | ||
dsu.transfer.whenCalledWith(liquidator.address, EXPECTED_LIQUIDATION_FEE.mul(1e12)).returns(true) | ||
dsu.balanceOf.whenCalledWith(market.address).returns(COLLATERAL.mul(1e12)) | ||
|
||
await market.connect(liquidator)['update(address,uint256,uint256,uint256,int256,bool)']( | ||
userB.address, | ||
0, | ||
0, | ||
0, | ||
0, | ||
true // Mark as protected | ||
) | ||
|
||
// 3. Move to next oracle version with invalid price | ||
const INVALID_ORACLE_VERSION = { | ||
...ORACLE_VERSION_3, | ||
valid: false | ||
} | ||
oracle.at.whenCalledWith(ORACLE_VERSION_3.timestamp).returns([INVALID_ORACLE_VERSION, INITIALIZED_ORACLE_RECEIPT]) | ||
oracle.status.returns([INVALID_ORACLE_VERSION, ORACLE_VERSION_4.timestamp]) | ||
oracle.request.whenCalledWith(userB.address).returns() | ||
|
||
// 4. Process the order - this should invalidate it but protection remains | ||
await settle(market, userB) | ||
|
||
// 5. Verify the vulnerability | ||
const order = await market.pendingOrders(userB.address, 2) | ||
expect(order.maker).to.eq(0) // Position should be zeroed | ||
expect(order.protection).to.eq(1) // Protection flag should be cleared but isn't | ||
|
||
// 6. This inconsistent state allows bypassing certain invariant checks | ||
await market.connect(userB)['update(address,uint256,uint256,uint256,int256,bool)']( | ||
userB.address, | ||
POSITION, | ||
0, | ||
0, | ||
0, | ||
false | ||
) | ||
|
||
// The operation succeeds when it should have failed due to protection status | ||
const finalOrder = await market.pendingOrders(userB.address, 3) | ||
expect(finalOrder.maker).to.eq(POSITION) // Should not be able to open position while protected | ||
}) | ||
}) | ||
``` | ||
|
||
### Mitigation | ||
|
||
Add protection flag reset in the `invalidate()` function: | ||
|
||
```solidity | ||
function invalidate(Order memory self, Guarantee memory guarantee) internal pure { | ||
self.protection = 0; // Reset protection flag | ||
(self.makerReferral, self.takerReferral) = | ||
(UFixed6Lib.ZERO, guarantee.orderReferral); | ||
// ... rest of the function | ||
} | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
Lucky Peach Barbel | ||
|
||
High | ||
|
||
# Division by Zero in Asset-to-Share and Share-to-Asset Conversion Functions Causes Complete Vault Operation Freeze and Fund Lock | ||
|
||
### **Root Cause** | ||
The issue stems from missing validation checks in the [_toSharesExact()](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/vault/contracts/types/Checkpoint.sol#L236-L238) and [_toAssetsExact()](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/vault/contracts/types/Checkpoint.sol#L232-L234) functions within `CheckpointLib`. These functions rely on the `self.assets` and `self.shares` variables to perform calculations involving division. Specifically: | ||
|
||
- In `_toSharesExact()`: | ||
```solidity | ||
return assets.muldiv(self.shares, UFixed6Lib.unsafeFrom(self.assets)); | ||
``` | ||
Here, the division `assets.muldiv(self.shares, UFixed6Lib.unsafeFrom(self.assets))` does not check if `self.assets` is zero. If `self.assets == 0`, the division operation causes a revert due to division by zero. | ||
|
||
- Similarly, in `_toAssetsExact()`: | ||
```solidity | ||
return shares.muldiv(UFixed6Lib.unsafeFrom(self.assets), self.shares); | ||
``` | ||
This function assumes that both `self.assets` and `self.shares` are non-zero. If `self.shares == 0`, the division will revert due to a division by zero error. | ||
|
||
- **`self.assets` Can Be Zero**: | ||
- This occurs when the vault has no assets remaining. This may happen due to high withdrawal fees, mismanagement of vault funds, or external market conditions reducing the collateral. | ||
|
||
- **`self.shares` Can Be Zero**: | ||
- This happens when the vault is fresh (no shares minted yet) or has been completely redeemed by all users, resulting in `self.shares == 0`. | ||
The lack of a guard clause (e.g. `require(self.assets > 0)` or `require(self.shares > 0)`) means: | ||
1. These division-by-zero scenarios are not handled gracefully. | ||
2. Any function calling `_toSharesExact()` or `_toAssetsExact()` will revert, halting execution and preventing user interactions with the vault. | ||
This issue is critical because it directly blocks user operations, essentially locking funds in the vault. | ||
|
||
--- | ||
|
||
### **Internal Pre-conditions** | ||
1. `self.assets` must be exactly 0 due to vault insolvency or high fees reducing the asset balance to zero. | ||
2. `self.shares` must be greater than 0, ensuring the division logic is triggered. | ||
|
||
--- | ||
|
||
### **External Pre-conditions** | ||
None. This issue arises purely from internal contract state mismanagement. | ||
|
||
--- | ||
|
||
### **Attack Path** | ||
1. A user interacts with a function that calls `_toSharesExact()` or `_toAssetsExact()` (e.g., converting assets to shares). | ||
2. If `self.assets` or `self.shares` is zero, the division operation will revert the transaction. | ||
3. All users attempting similar operations will face locked funds and inability to execute transactions. | ||
|
||
--- | ||
|
||
### **Impact** | ||
[toSharesGlobal()](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/vault/contracts/types/Checkpoint.sol#L155) | ||
```solidity | ||
return self.assets.lte(Fixed6Lib.ZERO) ? | ||
assets.unsafeSub(settlementFee) : | ||
_toShares(self, assets).unsafeSub(_toSharesExact(self, settlementFee)); | ||
``` | ||
If `self.assets == 0`, `_toSharesExact()` will revert. This blocks user deposits from being converted into shares, effectively halting all deposit operations globally. | ||
[toAssetsGlobal()](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/vault/contracts/types/Checkpoint.sol#L164) | ||
If `self.shares == 0`, `_toAssetsExact()` will revert. This blocks users from converting shares back to assets, freezing all redemption operations globally. | ||
```solidity | ||
return (self.shares.isZero() ? shares : _toAssets(self, shares)).unsafeSub(settlementFee); | ||
``` | ||
[complete()](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/vault/contracts/types/Checkpoint.sol#L101) | ||
```solidity | ||
self.shares = self.shares.add(profitShares); | ||
... | ||
profitShares = profitAssets.mul(self.shares).div(UFixed6Lib.unsafeFrom(self.assets).sub(profitAssets)); | ||
``` | ||
If `self.assets == 0`, the profit-sharing calculation in `complete()` will revert. This prevents the checkpoint from finalizing, locking all state updates and stopping vault operations. | ||
|
||
--- | ||
|
||
### **Mitigation** | ||
Add `require(self.assets > 0, "Assets cannot be zero")` and `require(self.shares > 0, "Shares cannot be zero")` checks before performing division in `_toSharesExact()` and `_toAssetsExact()`. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
Lucky Peach Barbel | ||
|
||
Medium | ||
|
||
# Oracle Manipulation Allows Execution of Invalid Orders | ||
|
||
### Description | ||
The [executeOrder](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/periphery/contracts/TriggerOrders/Manager.sol#L141) function in the `Manager` contract rely on the latest price fetched from the market's oracle (`market.oracle().latest()`) to determine whether an order's conditions are satisfied. This dependency exposes the protocol to oracle manipulation attacks, where an attacker could influence the reported price, either directly (e.g. by controlling or bribing the oracle) or indirectly (e.g. using a flash loan to temporarily alter the on-chain price used by the oracle). For instance, in the `checkOrder` function: | ||
```solidity | ||
canExecute = order.canExecute(market.oracle().latest()); | ||
``` | ||
A manipulated oracle price could incorrectly signal that an order is executable when it is not, enabling the attacker to execute orders under conditions that would normally not be met. This lack of price validation in the [latest](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/oracle/contracts/keeper/KeeperOracle.sol#L115-L117) function of the `KeeperOracle.sol` can lead to usage of stale `latestVersion` leaving the system vulnerable to malicious price swings during a transaction. | ||
|
||
### **Impact:** | ||
The primary impact is that attackers could exploit manipulated oracle prices to execute orders improperly, potentially draining user funds or destabilizing the system by executing invalid trades. | ||
|
||
### **Mitigation:** | ||
Add a staleness check in the `latest()` function to validate that the returned latestVersion is recent by comparing its timestamp with the current block timestamp and rejecting stale versions. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
Lucky Peach Barbel | ||
|
||
Medium | ||
|
||
# Premature Fee Deduction in `next.collateral` Update Leads to Incorrect Collateral Calculations | ||
|
||
In the `accumulate()` function of `CheckpointLib`,[the `next.collateral` calculation](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/libs/CheckpointLib.sol#L84-L86) prematurely subtracts settlement and trade fees from the latest checkpoint’s collateral before incorporating new collateral changes, price overrides, and other adjustments. Specifically, the code snippet below deducts `tradeFee` and `settlementFee` early in the calculation: | ||
|
||
```solidity | ||
next.collateral = settlementContext.latestCheckpoint.collateral | ||
.sub(settlementContext.latestCheckpoint.tradeFee) | ||
.sub(Fixed6Lib.from(settlementContext.latestCheckpoint.settlementFee)); | ||
``` | ||
|
||
This approach assumes that all fees are finalized and processed, which may not hold true if there are pending updates or deferred fee calculations in the [toVersion](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/libs/CheckpointLib.sol#L71). As a result, when the function subsequently adds the new collateral changes and price overrides (`result.collateral` and `result.priceOverride`), it can either double-deduct fees that are recomputed later or produce incorrect final values for `next.collateral`. This discrepancy creates potential inaccuracies in collateral tracking, particularly during edge cases involving concurrent settlement and collateral adjustments. | ||
|
||
#### **Impact**: | ||
The primary impact of this bug is under-collateralization, where the system misrepresents available collateral due to redundant or incomplete fee deductions, which can lead to insolvency or liquidation errors. | ||
|
||
#### **Mitigation**: | ||
Reorder the calculation of `next.collateral` so that settlement and trade fees are subtracted only after adding all new collateral, price override, and other adjustments. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
Raspy Black Giraffe | ||
|
||
Medium | ||
|
||
# SolverVault Coordinator Will Cause Losses for Vault Users via Unprotected Rebalancing | ||
|
||
### Summary | ||
|
||
The lack of slippage controls in the rebalance function will cause financial losses for vault users as the coordinator's transactions can be front-run or executed under unfavorable market conditions. | ||
|
||
### Root Cause | ||
|
||
In [SolverVault.sol](https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/a77aaa94d3b3a9725e4474428bc0a18ca2fde3b4/perennial-v2/packages/vault/contracts/SolverVault.sol#L40) there is no slippage checks | ||
```solidity | ||
function rebalance(IMarket from, IMarket to, UFixed6 amount) external onlyCoordinator { | ||
if (!_isRegistered(from) || !_isRegistered(to)) revert SolverVaultNotRegisteredError(); | ||
from.update(address(this), Fixed6Lib.ZERO, Fixed6Lib.from(-1, amount), address(0)); | ||
to.update(address(this), Fixed6Lib.ZERO, Fixed6Lib.from(1, amount), address(0)); | ||
} | ||
``` | ||
|
||
### Internal Pre-conditions | ||
|
||
1- Coordinator needs to call rebalance() to move funds between markets. | ||
2- Vault must hold sufficient collateral in the from market. | ||
|
||
### External Pre-conditions | ||
|
||
1- Market prices for the involved assets must be volatile | ||
2- The rebalance transaction is delayed, allowing market conditions to change unfavorably. | ||
|
||
### Attack Path | ||
|
||
1- Coordinator calls rebalance(fromMarket, toMarket, 1000 USDC) to adjust positions. | ||
|
||
2- Attacker detects the transaction in the mempool and front-runs it by: | ||
Artificially inflating the price of toMarket via a large buy order. | ||
Artificially deflating the price of fromMarket via a large sell order. | ||
|
||
3- The coordinator’s rebalance executes at manipulated prices: | ||
Vault sells fromMarket at a lower-than-expected price. | ||
Vault buys toMarket at a higher-than-expected price. | ||
|
||
### Impact | ||
|
||
Vault users suffer losses proportional to the manipulated price difference during rebalancing. For example: | ||
|
||
If the attacker causes a 5% price impact, users lose ~5% of the rebalanced amount. | ||
Attacker gains profit by closing their manipulated positions after the vault’s trade. | ||
|
||
### PoC | ||
|
||
```solidity | ||
// Simplified Foundry test showcasing front-running | ||
function testFrontrunRebalance() public { | ||
// 1. Setup: Vault holds 1000 USDC in MarketA | ||
_depositToVault(1000e6); | ||
// 2. Attacker manipulates MarketB’s price upward | ||
vm.startPrank(attacker); | ||
marketB.trade(attacker, 1_000_000e6); // Inflate price | ||
vm.stopPrank(); | ||
// 3. Coordinator’s rebalance executes at bad prices | ||
vm.prank(coordinator); | ||
solverVault.rebalance(marketA, marketB, 1000e6); | ||
// 4. Verify loss: Vault’s total assets decrease | ||
assertLt(solverVault.totalAssets(), 1000e6); | ||
} | ||
``` | ||
|
||
### Mitigation | ||
|
||
Add slippage controls to the rebalance function : | ||
```solidity | ||
function rebalance( | ||
IMarket from, | ||
IMarket to, | ||
UFixed6 amount, | ||
UFixed6 minAmountFrom, // Minimum received from `from` market | ||
UFixed6 minAmountTo, // Minimum received to `to` market | ||
uint256 deadline // Transaction expiry timestamp | ||
) external onlyCoordinator { | ||
if (block.timestamp > deadline) revert Expired(); | ||
(UFixed6 actualFrom, UFixed6 actualTo) = _executeRebalance(from, to, amount); | ||
if (actualFrom < minAmountFrom || actualTo < minAmountTo) revert SlippageExceeded(); | ||
} | ||
``` |
Oops, something went wrong.