Octane Security Review
Summary
Date: November 19, 2024 Auditor: Octane Security Project: Turbine Prediction Market System Contracts: Market.sol, Settlement.sol, ConditionalTokensWithPermit.sol
Issue Tracker
This document tracks security vulnerabilities discovered during the audit of the Turbine prediction market system, along with their fixes and test coverage.
Status Overview:
- 🔴 Critical Issues Found: 1
- 🟡 High Issues Found: 3
- 🔵 Low Issues Found: 4
- ℹ️ Informational Issues Found: 2
- 🟢 Medium Issues Found: 0
| Issue | Status |
|---|---|
| CRITICAL-01 | ✅ FIXED |
| HIGH-01 | ✅ FIXED |
| HIGH-02 | ✅ FIXED (separate PR — claimWinnings removed, redeemPositionsWithPermit added) |
| HIGH-03 | ✅ FIXED |
| LOW-01 | ✅ FIXED |
| LOW-02 | ✅ FIXED |
| LOW-03 | ✅ FIXED |
| LOW-04 | ⚠️ ACKNOWLEDGED (Architectural Trade-off) |
| INFORMATIONAL-01 | ✅ FIXED |
| INFORMATIONAL-02 | ✅ FIXED |
Critical Issues
CRITICAL-01: Incorrect Market Resolution Outcome Mapping
Status: ✅ FIXED
Severity: Critical
Contract: src/Market.sol
Severity Rationale:
- Impact: High — Incorrect market resolution leads to direct, material loss of principal funds for true winners (their tokens redeem 0) while losers redeem 1:1 from the collateral pool.
- Likelihood: High — Exploitation requires no special constraints beyond normal market participation.
Description: The Market contract did not store which outcome (YES/NO) was asserted when proposing a resolution to UMA. Instead, it directly mapped UMA's accepted/rejected boolean to YES/NO outcomes, which is incorrect. UMA's acceptance boolean only indicates whether the specific asserted statement is true — it does not inherently mean YES wins.
Exploitation Scenarios:
Scenario 1 (Critical): Real outcome is NO
- Before expiry: Attacker accumulates YES tokens cheaply
- After expiry: Honest user proposes NO assertion
- UMA accepts NO assertion as true
- Bug Impact:
settleMarket()maps acceptance to YES, reports payouts[1,0] - Result: Attacker redeems YES tokens for full value, NO holders (true winners) get nothing
Scenario 2 (High): Real outcome is YES
- Before expiry: Attacker accumulates NO tokens cheaply
- After expiry: Attacker proposes NO assertion (false claim)
- UMA rejects the false NO assertion
- Bug Impact:
settleMarket()maps rejection to NO, reports payouts[0,1] - Result: Attacker redeems NO tokens, YES holders get nothing
Truth Table (Buggy vs Fixed):
| Asserted | UMA Result | Correct Winner | Buggy Result | Status |
|---|---|---|---|---|
| YES | Accepted | YES | YES | ✅ |
| YES | Rejected | NO | NO | ✅ |
| NO | Accepted | NO | YES | ❌ CRITICAL |
| NO | Rejected | YES | NO | ❌ CRITICAL |
Fix: Store the asserted outcome when proposing resolution. Use assertedYes == accepted to determine the correct winner.
Fixed Truth Table:
| Asserted | Accepted | Logic | Winner | Status |
|---|---|---|---|---|
| YES (true) | Accepted (true) | true == true = true | YES | ✅ |
| YES (true) | Rejected (false) | false == true = false | NO | ✅ |
| NO (false) | Accepted (true) | true == false = false | NO | ✅ |
| NO (false) | Rejected (false) | false == false = true | YES | ✅ |
Test Coverage: 18/18 tests passing ✅
High Issues
HIGH-01: Resolution DoS via Repeated UMA Proposals
Status: ✅ FIXED
Severity: High
Contract: src/Market.sol
Description:
The proposeResolution() function lacked a guard to prevent multiple proposals. An attacker could repeatedly call proposeResolution() before the current assertion becomes settleable, overwriting marketData.umaAssertionId each time and indefinitely blocking market resolution.
Attack: Attacker proposes assertion A → before A's 2-hour liveness ends, proposes B (overwrites ID) → settleMarket() reverts → repeat indefinitely. Bond capital can be recycled by settling older assertions directly on UMA.
Cost Analysis:
- Bond Required: 100 USDC (configurable)
- Recurring Cost: $0 (bond fully recoverable)
- Attack Duration: Infinite
Fix: Added require(umaAssertionId == bytes32(0)) guard to enforce single-proposal-per-market invariant.
HIGH-02: claimWinnings() Context Bug
Status: ✅ FIXED (separate PR)
Severity: High
Contract: src/Market.sol (removed), src/ConditionalTokensWithPermit.sol (new function)
Description:
Market.claimWinnings() invoked CTF redemption from the Market contract's context. Because CTF's redeemPositions() uses msg.sender, the Market's balance was redeemed (typically zero), resulting in no payout while emitting a misleading success event.
Fix: Removed claimWinnings() entirely. Added redeemPositionsWithPermit() to ConditionalTokensWithPermit for gasless redemption via EIP-712 signatures, executing from the user's correct context.
Key Benefits:
- ✅ Correct context: Redeems from owner's address
- ✅ Gasless: Users sign off-chain, relayer pays gas
- ✅ Secure: EIP-712 signature validation with nonce replay protection
HIGH-03: 256-Outcome Condition Underflow
Status: ✅ FIXED
Severity: High
Contract: src/ConditionalTokensWithPermit.sol
Description:
The fullIndexSet calculation used (1 << outcomeSlotCount) - 1. When outcomeSlotCount == 256, the shift (1 << 256) causes an underflow in Solidity 0.8+, reverting all splitPosition(), mergePositions(), and redeemPositions() calls for 256-outcome conditions.
Fix: Changed to type(uint256).max >> (256 - outcomeSlotCount), which correctly handles all outcome counts including 256.
Test Coverage: 19/19 tests passing ✅
Low Issues
LOW-01: Inert Nonce Gate in OrderBook
Status: ✅ FIXED
Severity: Low
Contract: src/OrderBook.sol
Description:
The nonces mapping was never mutated, making the nonce check in validateOrder() effectively inert. Traders couldn't globally invalidate older orders.
Fix: Added cancelUpTo(N) function allowing traders to atomically invalidate all orders with nonce < N.
Test Coverage: 21/21 tests passing ✅
LOW-02: Reentrancy Cancel-and-Fill Inconsistency
Status: ✅ FIXED
Severity: Low
Contract: src/OrderBook.sol
Description:
_markFilled() didn't check if an order was cancelled before marking it as filled. A malicious contract could reentrantly cancel an order during ERC1155 transfer hooks, creating an inconsistent state where an order is both cancelled and filled.
Fix: Added cancellation check in _markFilled() — reverts if order was cancelled during execution.
LOW-03: Off-by-One Price Boundary Validation
Status: ✅ FIXED
Severity: Low
Contract: src/OrderBook.sol
Description:
validateOrder() used strict inequalities (<= and >=) for price boundary checks, incorrectly rejecting valid boundary prices of 1 and 999,999.
Fix: Changed <= to < and >= to > to accept the full valid price range [1, 999999].
Test Coverage: 23/23 tests passing ✅
LOW-04: Permissionless Trade Execution (Front-Running)
Status: ⚠️ ACKNOWLEDGED
Severity: Low
Contract: src/Settlement.sol
Description:
executeTrade() is permissionless — anyone can submit matched orders. Mempool observers can front-run by extracting orders from calldata and executing with alternate counterparty orders at less favorable prices.
Why Acknowledged (Not Fixed):
Restricting msg.sender would break the relayer/market maker model entirely. The permissionless design enables:
- Off-chain order matching with on-chain settlement
- Gas efficiency via relayer batching
- Professional market maker integration
Mitigations:
- Set tight price limits on orders
- Use short order expiry times
- Submit via trusted relayers with private mempools
- Use Flashbots/MEV-protection RPCs
Informational Issues
INFORMATIONAL-01: Empty disputedExplanation Field
Status: ✅ FIXED (Documentation)
Contract: src/Market.sol
getAssertionDetails() returns an empty disputedExplanation because UMA V3 doesn't expose dispute reasons on-chain. Documentation updated to clarify this and direct callers to use the disputer address field instead.
INFORMATIONAL-02
Status: ✅ FIXED