Skip to content

Latest commit

 

History

History
362 lines (307 loc) · 29.9 KB

File metadata and controls

362 lines (307 loc) · 29.9 KB

PredictIQ Contract Analysis: Comprehensive Issues & Vulnerabilities Report

This report provides a granular analysis of 50 identified issues within the PredictIQ Soroban smart contract suite. Each issue is detailed with its severity, location, impact, and a recommended mitigation path.


Detailed Issue Analysis (1-50)

Issue 1: Multi-Token Referral Reward Mixing

File: fees.rs Severity: Critical Description: The ReferrerBalance map uses only the referrer's Address as a key, storing a single i128 balance. Since the contract allows betting in multiple different tokens (e.g., USDC, XLM, BTC), all referral commissions are summed into this one balance without regard for the underlying asset. Impact: A user who earns 100 XLM in referral rewards could call claim_referral_rewards providing a USDC token address. The contract would transfer 100 USDC to them, effectively allowing unauthorized cross-asset conversion and potential draining of high-value pools. Mitigation: Update the storage key to a tuple of (Address, Address) representing (Referrer, Token).

Issue 2: Non-Parimutuel Payout Logic

File: bets.rs Severity: Critical Description: The implementation in claim_winnings contains let winnings = bet.amount;. This logic only returns the user's original stake upon winning, failing to distribute the pool of losing bets. Impact: The core value proposition of a prediction market—where winners split the pool of losers—is broken. This removes all financial incentive for participants and leads to "trapped" capital (losers' stakes) that can never be claimed. Mitigation: Implement pool-ratio based payout math: winnings = (bet.amount * total_staked) / winning_outcome_stake.

Issue 3: Missing Governance Token Configuration Enum

File: voting.rs Severity: High Description: The contract attempts to read ConfigKey::GovernanceToken in cast_vote, but this variant is missing from the ConfigKey enum definition in types.rs. Impact: Immediate compilation failure. Even if patched manually, the lack of a standardized config key prevents governance initialization. Mitigation: Add GovernanceToken to the ConfigKey enum in types.rs.

Issue 4: Hardcoded Voting Power Calculation

File: governance.rs Severity: High Description: The majority threshold for contract upgrades is calculated based on the number of guardians rather than their assigned voting_power. Impact: This renders the voting_power field in the Guardian struct useless and forces a "one person, one vote" model which may not align with the intended economic stakes. Mitigation: Update is_majority_met to sum voting_power of voters and compare against total possible power.

Issue 5: References to Non-existent Error Codes

File: sac.rs Severity: High Description: The contract references error codes like ErrorCode::AssetClawedBack and ErrorCode::StalePrice that are not defined in the errors.rs manifest. Impact: Compilation failure. If logic depends on these specific error paths for safety (e.g., stopping execution on clawback), the system is currently unbuildable. Mitigation: Synchronize errors.rs with all error variants used across the module suite.

Issue 6: Missing Deadline vs Resolution Deadline Validation

File: markets.rs Severity: Medium Description: create_market does not verify that the betting deadline is strictly before the resolution_deadline. Impact: Markets can be created where participants can place bets after the event's outcome might already be known or during the resolution phase, leading to arbitrage and disputes. Mitigation: Add a requirement check: require!(deadline < resolution_deadline).

Issue 7: Premature Release of Creator Deposits

File: markets.rs Severity: Medium Description: Creation deposits are released immediately upon any resolution, regardless of whether the resolution was contested or if the creator acted maliciously. Impact: There is zero economic penalty for bad actors who intentionally create fraudulent markets, as their deposit is returned even if the market is disputed. Mitigation: Only release deposits after a "Dispute Window" has closed without a successful challenge.

Issue 8: Narrow Dispute Window for Global Participation

File: resolution.rs Severity: Medium Description: The hardcoded DISPUTE_WINDOW_SECONDS is set to 24 hours. Impact: For a global prediction market, 24 hours is insufficient for participants in different time zones to detect a bad oracle result and coordinate a dispute, especially over weekends. Mitigation: Increase the default window to 48-72 hours or make it a configurable parameter.

Issue 9: Global Outcome Key Collisions in Oracles

File: oracles.rs Severity: Medium Description: get_oracle_result uses a hardcoded sub-index 0 for storing results: OracleData::Result(market_id, 0). Impact: While safe per market, this restricts the system to only one oracle result per market forever, preventing multi-oracle aggregation or historical retrieval if a market is re-resolved. Mitigation: Incorporate an oracle_id or version into the OracleData key structure.

Issue 10: Master Admin Role Bloat

File: lib.rs Severity: Medium Description: High-frequency operational tasks (like setting market tiers) and high-risk tasks (like contract upgrades) all share the same Admin role check. Impact: Increased "blast radius" for a single compromised key. The master admin is forced into frequent on-chain activity, increasing exposure. Mitigation: properly utilize the MarketAdmin and FeeAdmin roles for operational tasks, reserving Admin for structural changes.

Issue 11: Reliance on Host Panics for SAC Transfers

File: sac.rs Severity: Medium Description: safe_transfer relies on the underlying Soroban environment to panic if a transfer fails, rather than handling a Result from the token contract. Impact: Prevents the contract from implementing "graceful failure" logic or detailed error events when a transfer is blocked by a frozen asset or insufficient balance. Mitigation: Use e.try_invoke_contract for token transfers to catch and handle errors programmatically.

Issue 12: Circuit Breaker Lack of Recovery Path

File: circuit_breaker.rs Severity: Medium Description: The automated circuit breaker can trigger an Open state, but there is no programmatic logic for transitioning back to Closed or Half-Open based on health checks. Impact: The system remains bricked until an admin manually intervenes, even if the underlying volatility or error condition has subsided. Mitigation: Implement a "Cool-down" period after which the state automatically transitions to Half-Open.

Issue 13: Hardcoded Timelock Inflexibility

File: types.rs Severity: Low Description: The 48-hour TIMELOCK_DURATION for upgrades is a hardcoded constant. Impact: Emergency patches are delayed by 48 hours regardless of urgency, while major architectural shifts cannot be granted a longer review period. Mitigation: Make the timelock a configurable parameter with a governance-defined minimum/maximum.

Issue 14: Stubbed Analytics Counter

File: markets.rs Severity: Low Description: count_bets_for_outcome ignores the actual bet data and returns a constant 1 or 0 based on key existence. Impact: Front-ends and analytical tools receive incorrect data regarding market participation, leading to a poor user experience and "ghost" markets. Mitigation: Implement a proper counter in DataKey::OutcomeBetCount(market_id, outcome) updated during place_bet.

Issue 15: Event Topic Naming Inconsistency

File: fees.rs Severity: Low Description: Different modules use varying methods for event names (e.g., symbol_short! vs Symbol::new). Impact: Off-chain indexers may miss certain events or require custom logic for every module, increasing the fragility of the data pipeline. Mitigation: Move all event emission to a central events.rs module with standardized naming conventions.

Issue 16: OracleConfig Struct Schema Mismatch

File: types.rs Severity: Medium Description: The OracleConfig struct is missing fields like max_staleness_seconds and max_confidence_bps that are actively used in oracles.rs. Impact: Hard compilation error when building the contract, as the logic attempts to access non-existent struct fields. Mitigation: add the missing fields to the OracleConfig struct definition in types.rs.

Issue 17: Pruning Without Reward Verification

File: markets.rs Severity: Medium Description: The prune_market function deletes market data based solely on time passed since resolution. Impact: If winners have not claimed their rewards within 30 days, their records are deleted, and their funds become permanently trapped in the contract. Mitigation: Add a check to ensure total_claimed == total_staked before allowing a market to be pruned.

Issue 18: Non-Functional Upgrade Execution Chain

File: governance.rs Severity: High Description: execute_upgrade validates all conditions but returns the WASM hash instead of calling the host's update_current_contract_wasm. Impact: The governance process finishes "successfully" but the contract code is never actually updated. The system is in a "stuck" state where users think an upgrade happened but it didn't. Mitigation: Invoke e.deployer().update_current_contract_wasm(wasm_hash) within the execute_upgrade function.

Issue 19: Missing Admin/Guardian Integrity Check

File: governance.rs Severity: Medium Description: add_guardian does not prevent the master Admin address from being added to the guardian set. Impact: Allows a single entity to bypass the checks and balances of the multi-party system by acting as both the initiator and the majority voter. Mitigation: Add require!(guardian.address != admin) in add_guardian.

Issue 20: Time-Based Vote Token Unlocking

File: voting.rs Severity: Medium Description: unlock_tokens allows users to retrieve their voting tokens strictly based on a timestamp (unlock_time), regardless of whether the market has actually been resolved. Impact: Voters can withdraw their weight during an active dispute if the resolution is delayed, potentially leading to a "No Majority" state where the market cannot be closed. Mitigation: Require market.status == Resolved before allowing governance tokens to be unlocked.

Issue 21: Referrer Address Auth Bypass

File: bets.rs Severity: Low Description: The referrer parameter in place_bet is accepted without verification. Impact: A malicious actor can provide their own address as a referrer for every bet they place, effectively getting a systemic discount on fees that they shouldn't be entitled to. Mitigation: Add a check to ensure referrer != bettor.

Issue 22: Conditional Market Workflow Deadlock

File: markets.rs Severity: Medium Description: create_market requires the parent_id market to be already in Resolved status before a child market can be created. Impact: Prevents the creation of "Prediction Parleys" or complex tree-based markets (e.g., Round 1 and Round 2 of a tournament) from being set up in advance. Mitigation: Allow creation in Active status but block place_bet on the child market until the parent resolves.

Issue 23: Forced Resolution Mode Switching

File: disputes.rs Severity: Medium Description: The system automatically switches a market to Pull or Push mode during resolution based on a heuristic. Impact: If a market was initially set as Push (for automated distribution) but resolution forces it to Pull, users might be confused as to why they haven't received funds automatically. Mitigation: Payout mode should be immutable after market creation to ensure predictable user flows.

Issue 24: Unsafe 100-Unit Average Bet Heuristic

File: disputes.rs Severity: High Description: estimate_winner_count uses a fixed (tally / 100) logic to estimate the number of winners to calculate gas limits. Impact: If the average bet is 1 unit (e.g., 1 micro-USDC), the actual winner count could be 100x higher than estimated, causing the Push resolution to blow through the gas limit and fail. Mitigation: Maintain an actual u32 winner counter per outcome updated during place_bet.

Issue 25: Stubbed Pyth Oracle Integration

File: oracles.rs Severity: High Description: The fetch_pyth_price function always returns an OracleFailure error. Impact: The automated resolution path is completely broken in the current codebase. Any market relying on Pyth for resolution will remain stuck in Active mode forever. Mitigation: Replace the stub with a real Pyth contract cross-contract call.

Issue 26: Protocol Revenue Withdrawal Gap

File: fees.rs Severity: Medium Description: While the contract correctly tracks FeeRevenue in its internal state, it lacks a dedicated function for the treasury or admin to withdraw these accumulated tokens. Impact: Protocol revenue is effectively "burned" or stuck in the contract balance. There is no programmatic way to transfer collected fees to a cold wallet or DAO treasury. Mitigation: Implement a withdraw_protocol_fees function restricted to the FeeAdmin or Admin role.

Issue 27: Usage of Non-Existent SAC Error Codes

File: sac.rs Severity: Medium Description: The detect_clawback function attempts to return ErrorCode::AssetClawedBack, which is missing from the global error enum. Impact: Immediate runtime crash or compilation failure depending on the build environment. This prevents the contract from safely handling clawback-enabled Soroban assets. Mitigation: add AssetClawedBack to the ErrorCode enum in errors.rs.

Issue 28: Unprotected Contract Initialization

File: lib.rs Severity: High Description: The initialize function can be called by anyone as long as the contract hasn't been initialized yet. Impact: An attacker can front-run the developer's initialization transaction on-chain, setting themselves as the Admin and taking full control of the project immediately after deployment. Mitigation: Implement a "deployer-only" check in the initialize function using e.deployer().get_current_contract_address().

Issue 29: Structural Passthrough Wrapper Bloat

File: lib.rs Severity: Low Description: The main PredictIQ struct acts as a stateless wrapper for dozens of internal modules, duplicating function signatures for every exposed method. Impact: Unnecessary increase in WASM binary size and deployment costs. It also makes the code harder to maintain as every change must be mirrored in the root file. Mitigation: Utilize Soroban's cross-contract calling or internal module exposure directly where appropriate to reduce boilerplate.

Issue 30: Arbitrary Gas-Limit Mode Switching

File: disputes.rs Severity: Low Description: The 50-winner threshold for switching to Push mode is a hardcoded constant and does not account for the varying gas costs of different token implementations (e.g., custom SACs). Impact: Markets with 40 winners using a gas-intensive token might still exceed the ledger limit in Push mode, bricking the resolution transaction. Mitigation: Make MAX_PUSH_PAYOUT_WINNERS a configurable parameter in OracleConfig or MarketTier.

Issue 31: Bloated Market Struct with In-Memory Maps

File: markets.rs Severity: Medium Description: The Market struct stores outcome_stakes as a Map<u32, i128> directly inside the persistent struct. Impact: Every time a market is fetched (get_market), the entire map of outcome stakes is deserialized. For markets with 100 outcomes, this leads to linear scaling of gas costs for simple reads. Mitigation: Store outcome stakes in separate storage keys (e.g., DataKey::OutcomeStake(market_id, outcome)) to allow constant-time access.

Issue 32: WASM Hash Type Mismatch in Governance

File: governance.rs Severity: High Description: PendingUpgrade stores the wasm_hash as a String, but Soroban host functions for upgrades require BytesN<32>. Impact: The upgrade will fail at the execution step because hex-string conversion to bytes is not handled, or because the data type is incompatible with the host environment. Mitigation: Change the wasm_hash field to BytesN<32>.

Issue 33: Governance Data Tuple Return Risk

File: governance.rs Severity: Low Description: get_upgrade_votes returns statistics as a raw tuple (u32, u32). Impact: External integrations (like a DAO dashboard) are prone to "index confusion," where "For" and "Against" votes are swapped, leading to incorrect governance displays for users. Mitigation: Return a named struct UpgradeStats { votes_for: u32, votes_against: u32 }.

Issue 34: Unused Administration Code Bloat

File: admin.rs Severity: Low Description: Several functions like get_market_admin and get_fee_admin are defined but never utilized by the core logic in markets.rs or fees.rs. Impact: Higher deployment costs and noise for security reviewers. Mitigation: Remove unused "getter" functions unless they are explicitly required for the external API.

Issue 35: Incomplete Resolution Event Metadata

File: disputes.rs Severity: Low Description: The ResolutionFinalized event has a hardcoded 0 for the total payout amount. Impact: Off-chain indexers and analytics platforms cannot track the total economic volume of resolved markets without performing expensive secondary lookups. Mitigation: calculate the total payout based on winning outcome stakes and include it in the event data.

Issue 36: State Durability (TTL) Mismatch

File: types.rs Severity: Medium Description: The PRUNE_GRACE_PERIOD is 30 days, which is identical to the TTL_HIGH_THRESHOLD (~30 days in ledgers). Impact: If a market is not frequently bumped, its persistent data might expire exactly when the pruning window opens, causing the prune_market admin function to fail or find "MarketNotFound". Mitigation: Increase the high TTL threshold to 60-90 days to ensure data lives safely through the pruning grace period.

Issue 37: Token Balance Tracking Loss during Fallback

File: voting.rs Severity: High Description: The fallback mechanism for tokens without snapshot support locks tokens in the contract but doesn't maintain an internal ledger of which user owns which portion of the locked pool. Impact: If multiple users lock the same token for the same market, the contract cannot distinguish their balances, risking "First-come, first-served" withdrawals where early claimers drain the pool. Mitigation: Maintain a Map<Address, i128> for locked balances per market.

Issue 38: Inconsistent Storage Layer Segregation

File: monitoring.rs Severity: Medium Description: Circuit breaker logic resides in persistent storage while error frequency trackers are stored in instance storage. Impact: If the contract's instance expires (but persistent data lives on), the circuit breaker could stay "Open" while the error counters reset to zero, creating an inconsistent and unrecoverable system state. Mitigation: Move all monitoring and circuit breaker state variables to the same storage level (Instance).

Issue 39: Precision Loss in Multi-Tier Fee Math

File: fees.rs Severity: Medium Description: Tiered fee multipliers use integer-based division for percentages (75/100, 50/100) before multiplying by the principle. Impact: For small betting amounts or 1-unit tokens, the discount calculation can truncate down to zero, effectively charging the same fee to All tiers. Mitigation: Reorder arithmetic to multiply first: (amount * multiplier) / 100.

Issue 40: Logic Duplication in Claim Paths

File: bets.rs Severity: Low Description: The logic for claim_winnings and withdraw_refund shares 90% of the same code path, including token transfers and record deletion. Impact: Increased risk of "divergent bugs" where a fix is applied to one path but forgotten in the other. Mitigation: Refactor into a private internal_claim_amount helper function.

Issue 41: Absolute Price Confidence Sign-Check Missing

File: oracles.rs Severity: Medium Description: The absolute value conversion for Pyth prices doesn't account for the potential for overflow when dealing with i64::MIN. Impact: Although rare in pricing, an extremely large negative price could cause a contract panic during confidence verification, blocking resolution. Mitigation: Use saturating_abs() or explicit bounds checking.

Issue 42: Governance Upgrade Collision Lack of Check

File: lib.rs Severity: Medium Description: The system allows starting a second upgrade vote for a WASM hash that is already pending or even recently rejected. Impact: Spam and confusion for the Guardian set. Attackers can flood the governance queue with repetitive or slightly modified upgrade requests. Mitigation: Block initiate_upgrade if a pending upgrade exists or if the same hash was rejected within a cool-down period.

Issue 43: Admin Control over Guardian Autonomy

File: governance.rs Severity: High Description: The Admin role can remove all Guardians at any time. Impact: This renders the "multi-party governance" a facade. The Admin can simply remove all dissenting Guardians, add their own secondary accounts, and instantly approve any malicious upgrade. Mitigation: implement a "Guardian Majority" requirement for the removal of any existing Guardian.

Issue 44: Silent Reset of Automated Monitoring

File: monitoring.rs Severity: Low Description: Resetting the automated circuit breaker counters does not trigger an on-chain event. Impact: Dev ops and community auditors have no visibility into how often the system is hitting error thresholds unless they are actively polling the state. Mitigation: Emit a MonitorReset event during counter clearing.

Issue 45: Binary Evolution of Creator Reputation

File: markets.rs Severity: Low Description: The CreatorReputation system is used as a binary switch (Pro/Inst vs None/Basic) for deposits. Impact: There is no incentive for a "Basic" user to grow their reputation, as they receive no incremental benefits until they hit the "Pro" threshold. Mitigation: Implement a linear deposit discount based on exact reputation points.

Issue 46: PayoutMode Documentation-Implementation Gap

File: types.rs Severity: Low Description: PayoutMode documentation describes logic for automated distribution that is entirely missing from the resolution.rs implementation. Impact: Integration developers reading the types might assume the feature is functional and build front-ends that fail to provide "Claim" buttons for users. Mitigation: Update docs to reflect current status or implement the missing distribution logic.

Issue 47: Manual Market Lifecycle Pruning

File: markets.rs Severity: Low Description: Pruning market data requires an active administrative transaction. Impact: If the admin team becomes inactive, the contract will accumulate technical debt and storage costs indefinitely. Mitigation: Make prune_market a permissionless function that anyone can call after the grace period expires.

Issue 48: Initialization Failure Lock-in Risk

File: governance.rs Severity: High Description: If initialize_guardians is not called during the same transaction as contract deployment, the contract can end up in a state with an Admin but NO Guardians. Impact: Since contact upgrades require Guardian approval, and the Admin can't add Guardians if the set is empty (logic error in some paths), the contract becomes un-upgradable forever. Mitigation: Force Guardian initialization within the main initialize call.

Issue 49: Signed vs Unsigned Timestamp Comparisons

File: oracles.rs Severity: Medium Description: Timestamp logic in oracle validation mixes u64 (ledger time) with i64 (Pyth publication time). Impact: Risk of integer underflow during comparisons if an oracle result has a future timestamp or if the ledger clock is slightly out of sync. Mitigation: Unify all internal time logic to u64 and cast Pyth data with safety checks.

Issue 50: Lack of Global Community "Panic" Override

File: lib.rs Severity: Medium Description: The system lacks a mechanism for the community (or a majority of Guardians) to override the Admin and pause the contract. Impact: If the Admin address is hijacked, the hijackers can keep the contract running to drain funds, and the Guardians are powerless to stop it. Mitigation: allow a 2/3 Guardian majority to trigger the Paused state without Admin consent.


9. Conclusion

The PredictIQ codebase demonstrates significant architectural foresight but is currently not production-ready. The combination of multi-token accounting failures, incorrect payout mathematics, and incomplete governance execution paths represents a critical risk profile that requires immediate remediation.