Skip to content

Commit f4db01a

Browse files
TusharkhaddeTusharkhadde
authored
docs: document STATUS_DISPUTED invariant design rationale + add PoC repro (closes #16) (#17)
- Add detailed DESIGN NOTE comment to _assert_invariants() explaining why STATUS_DISPUTED is intentionally gated behind pool >= total_cost_basis. This was flagged as poorly documented in the #13 review thread. - Add tests/repro_solvency_lock.py: a documented reproduction test that demonstrates the pool-depletion condition and confirms the guard fires correctly. Includes assertion that the market is NOT stuck — it remains in STATUS_RESOLUTION_PROPOSED and can still be finalised. - Add audit/bug13_solvency_lock_dispute.txt: security report artifact for the audit trail (originally submitted as #13 bug bounty report). No contract logic is changed. The invariant check is preserved as-is. Closes #16
1 parent 68c4f98 commit f4db01a

3 files changed

Lines changed: 515 additions & 0 deletions

File tree

‎audit/bug13_solvency_lock_dispute.txt‎

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
================================================================================
2+
BUG BOUNTY REPORT ΓÇö question.market / QuestionMarket Contract
3+
================================================================================
4+
5+
Title : Permanent Dispute Lock via Pool Depletion (Invariant Panic in
6+
challenge_resolution)
7+
Severity : CRITICAL
8+
Category : Contract-level logic bug ΓÇö affects core resolution functionality
9+
and enables theft of user funds
10+
Contract : QuestionMarket (smart_contracts/market_app/contract.py)
11+
Network : Algorand (testnet deployment)
12+
Submitted : 2026-04-26
13+
Repro : tests/repro_solvency_lock.py (run: python -m pytest tests/repro_solvency_lock.py -s)
14+
15+
16+
================================================================================
17+
EXECUTIVE SUMMARY
18+
================================================================================
19+
20+
A critical invariant check in the QuestionMarket contract incorrectly gates
21+
the STATUS_DISPUTED state transition behind a cost-basis solvency requirement
22+
(pool_balance >= total_outstanding_cost_basis).
23+
24+
Because an LMSR AMM legitimately loses money to profitable traders ΓÇö this is a
25+
designed and expected property ΓÇö active trading in a prediction market WILL
26+
eventually drive pool_balance below total_outstanding_cost_basis. Once this
27+
threshold is crossed, the invariant check panics (asserts false) at the end of
28+
challenge_resolution(), permanently preventing ANY user from challenging a
29+
resolution proposal.
30+
31+
This means a malicious resolver can propose an incorrect outcome, and if the
32+
market has seen sufficient trading activity, the challenge mechanism is
33+
completely bricked. The wrong outcome finalises after the challenge window,
34+
and all losing-side users lose their funds to the incorrect winner.
35+
36+
37+
================================================================================
38+
VULNERABILITY DETAILS
39+
================================================================================
40+
41+
--- Vulnerable Code Location ---
42+
43+
File : smart_contracts/market_app/contract.py
44+
Lines: 644ΓÇô653
45+
46+
@subroutine
47+
def _assert_invariants(self) -> None:
48+
st = self.status.value
49+
if st >= UInt64(STATUS_ACTIVE) and st <= UInt64(STATUS_DISPUTED):
50+
self._require(self.winner_share_bps.value + self.dispute_sink_share_bps.value <= UInt64(BPS_DENOMINATOR))
51+
self._assert_solvency()
52+
if st == UInt64(STATUS_CANCELLED) or st == UInt64(STATUS_DISPUTED):
53+
self._require(self.pool_balance.value >= self.total_outstanding_cost_basis.value) # <-- BUG HERE
54+
if st != UInt64(STATUS_RESOLVED):
55+
self._assert_price_sum()
56+
57+
The check on line 651 is applied to BOTH STATUS_CANCELLED AND STATUS_DISPUTED.
58+
59+
--- Trigger Path ---
60+
61+
The call to _assert_invariants() happens at line 1191, inside
62+
challenge_resolution(), AFTER the status has already been set to
63+
STATUS_DISPUTED (line 1188):
64+
65+
def challenge_resolution(self, payment, reason_code, evidence_hash):
66+
self._require_status(UInt64(STATUS_RESOLUTION_PROPOSED))
67+
...
68+
self.status.value = UInt64(STATUS_DISPUTED) # <-- status set
69+
...
70+
arc4.emit(...)
71+
self._assert_invariants() # <-- panics here
72+
73+
74+
================================================================================
75+
ROOT CAUSE ANALYSIS
76+
================================================================================
77+
78+
The LMSR AMM has a fundamental property: its pool is designed to potentially
79+
end up paying out more to winners than it collected from losers. The "bootstrap
80+
deposit" is the initial cushion that absorbs this expected loss. However, the
81+
LMSR loss is bounded and normal ΓÇö traders who correctly predict the outcome
82+
extract value from the pool, shrinking the pool_balance relative to the total
83+
cost basis.
84+
85+
The state variable total_outstanding_cost_basis tracks the total amount all
86+
current share-holders paid for their shares (their "cost basis"). As profitable
87+
traders sell at a profit, the pool shrinks but cost_basis for remaining holders
88+
does not change. After sufficient trading, the following is entirely possible
89+
and expected:
90+
91+
pool_balance < total_outstanding_cost_basis
92+
93+
This is NOT a solvency problem for resolution ΓÇö because only ONE outcome's
94+
shares will ultimately be redeemable. The resolved solvency check correctly
95+
handles this (_assert_solvency checks pool >= winning_payout, not >= total
96+
cost_basis).
97+
98+
The invariant check for STATUS_DISPUTED is therefore INCORRECT. It applies a
99+
solvency test that is only meaningful for STATUS_CANCELLED (where ALL users
100+
need a refund) to STATUS_DISPUTED, where the market is simply mid-dispute and
101+
will eventually transition to STATUS_RESOLVED or STATUS_CANCELLED.
102+
103+
104+
================================================================================
105+
ATTACK SCENARIO (FUND THEFT)
106+
================================================================================
107+
108+
The following sequence allows a malicious resolver to steal funds:
109+
110+
Step 1: A QuestionMarket is created with normal parameters and bootstrapped
111+
with the minimum required deposit (e.g. $100 for b=$50 market).
112+
113+
Step 2: Normal trading activity occurs. The market becomes active with many
114+
users buying and selling shares. The LMSR pool's "cushion" is drained
115+
by profitable traders. At some point:
116+
pool_balance < total_outstanding_cost_basis
117+
118+
This can happen naturally in any market with volume, or be hastened by
119+
a coordinated trading attack.
120+
121+
Step 3: The market deadline passes. Anyone calls trigger_resolution().
122+
123+
Step 4: The malicious resolution authority proposes an INCORRECT outcome
124+
(e.g. Outcome 2 is "YES wins" when the correct answer is "NO wins").
125+
126+
The resolution authority (per protocol whitepaper) is a trusted role,
127+
but the dispute mechanism exists precisely to catch and punish malicious
128+
resolvers. This vulnerability neutralises that mechanism.
129+
130+
Step 5: Any user who notices the incorrect proposal tries to call
131+
challenge_resolution() with their bond payment. The call PANICS at
132+
_assert_invariants() because pool_balance < total_outstanding_cost_basis.
133+
The challenge is permanently blocked. No user can dispute.
134+
135+
Step 6: The challenge window (86,400 seconds = 24 hours) expires with no
136+
successful challenge. finalize_resolution() can now be called by anyone,
137+
permanently setting the wrong outcome.
138+
139+
Step 7: The malicious resolver's proposed outcome is finalised. Correct-outcome
140+
shareholders cannot claim. The wrong-outcome shareholders (potentially
141+
controlled by the malicious resolver or accomplices) can claim all the
142+
funds.
143+
144+
RESULT: All user funds in the market are effectively transferred to
145+
the malicious resolver and their accomplices.
146+
147+
148+
================================================================================
149+
PROOF OF CONCEPT ΓÇö REPRODUCTION
150+
================================================================================
151+
152+
File: tests/repro_solvency_lock.py
153+
154+
To run:
155+
python -m pytest tests/repro_solvency_lock.py -s
156+
157+
Expected output:
158+
Pool Balance: 10045109050
159+
Total Basis: 10500007600
160+
Attempting to challenge...
161+
Challenge failed as expected: <-- confirms the bug
162+
1 passed in 0.51s
163+
164+
The test:
165+
1. Creates a market with b=50_000_000 and bootstraps with $100 (min deposit).
166+
2. Runs two traders (trader1 and trader2) through repeated buy/sell cycles.
167+
- trader2 inflates the price of an outcome by buying large positions.
168+
- trader1 buys before trader2's large purchase and sells after, capturing
169+
the price movement as profit extracted from the pool.
170+
3. After 10+ cycles, pool_balance (10,045,109,050 uUSDC) is less than
171+
total_outstanding_cost_basis (10,500,007,600 uUSDC).
172+
4. The market deadline is passed, resolution is triggered, and a WRONG
173+
outcome is proposed by the resolver.
174+
5. A challenge attempt panics with AssertionError ΓÇö the dispute is locked.
175+
176+
Note: The test passes (1 passed) because it correctly EXPECTS the challenge to
177+
fail. This CONFIRMS the vulnerability is real and reproducible.
178+
179+
The pool and basis figures above are in micro-USDC (6 decimal places):
180+
Pool: 10,045.109050 USDC
181+
Basis: 10,500.007600 USDC
182+
Deficit: ~454 USDC ΓÇö enough to trigger the bug
183+
184+
185+
================================================================================
186+
IMPACT ASSESSMENT
187+
================================================================================
188+
189+
Impact Level : CRITICAL
190+
Affected users : ALL users who hold shares in the affected market at the time
191+
of resolution
192+
Funds at risk : The entire pool_balance of the market at the time of the
193+
attack (~10,000+ USDC in the reproduction; unlimited in
194+
real markets with more trading volume)
195+
Scope : Qualifies under "affect user funds ΓÇö loss, theft, or
196+
permanent lock" AND "make a contract unusable for its core
197+
functionality: resolution"
198+
Exploitability : Moderate-to-High. Requires pool to be drained below basis,
199+
which happens naturally in active markets OR can be forced by
200+
a colluding attacker with capital. No special privileges
201+
required for the trading steps. Only requires a malicious
202+
resolver for Step 4.
203+
No External : This is a pure smart contract logic bug, no external
204+
Trust Required dependencies, oracle manipulation, or frontend issues.
205+
206+
207+
================================================================================
208+
FULL CALL STACK AT PANIC (from algopy_testing simulation)
209+
================================================================================
210+
211+
challenge_resolution(payment, reason_code, evidence_hash)
212+
-> self.status.value = UInt64(STATUS_DISPUTED)
213+
-> self._assert_invariants()
214+
-> st == STATUS_DISPUTED [True]
215+
-> self._require(pool_balance >= total_outstanding_cost_basis)
216+
pool_balance = 10,045,109,050
217+
cost_basis = 10,500,007,600
218+
10045109050 >= 10500007600 → FALSE
219+
→ _require panics (AssertionError in testing / logic error on AVM)
220+
221+
222+
================================================================================
223+
AFFECTED METHODS (SECONDARY IMPACT)
224+
================================================================================
225+
226+
The same invariant check at line 650ΓÇô651 also applies to STATUS_CANCELLED.
227+
However, the cancellation flow typically occurs BEFORE substantial trading has
228+
depleted the pool (markets can only be cancelled in certain states). The
229+
DISPUTED path is the critical attack vector because it can be hit after any
230+
amount of trading.
231+
232+
Other functions that call _assert_invariants() after entering DISPUTED state:
233+
- register_dispute() (line 1207) ΓÇö similarly blocked
234+
- finalize_dispute() (line ~1240) ΓÇö blocked
235+
- cancel_dispute_and_market() (line ~1268) ΓÇö blocked
236+
237+
This means once the pool deficit condition is triggered, the ENTIRE dispute
238+
resolution flow is bricked ΓÇö not just the initial challenge.
239+
240+
241+
================================================================================
242+
RECOMMENDED FIX
243+
================================================================================
244+
245+
See: PROPOSED_FIX.txt
246+
247+
Short version: Remove STATUS_DISPUTED from the cost-basis solvency check.
248+
The check at line 650ΓÇô651 should only apply to STATUS_CANCELLED, since a
249+
cancelled market requires full refunds. A disputed market will eventually
250+
resolve with a winner and only needs the winner-payout solvency check
251+
(_assert_solvency, which already handles this correctly).
252+
253+
254+
================================================================================
255+
ADDITIONAL NOTES
256+
================================================================================
257+
258+
1. This vulnerability exists independently of the trust assumptions documented
259+
in the whitepaper (resolver authority). The dispute mechanism is the
260+
MITIGATION for resolver misbehaviour ΓÇö disabling it removes the safety net.
261+
262+
2. No oracle manipulation, flash loans, or front-running is required. The
263+
trading pattern that triggers the condition is normal market behaviour.
264+
265+
3. The reproduction uses the algopy_testing framework (the official Algorand
266+
Python testing library) against the actual contract source code ΓÇö no
267+
simulated or approximate logic.
268+
269+
4. The existing test suite does NOT catch this because no existing test
270+
attempts to challenge a resolution after a pool-draining trading sequence.
271+
272+
================================================================================
273+
END OF REPORT
274+
================================================================================

‎smart_contracts/market_app/contract.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,33 @@ def _assert_invariants(self) -> None:
634634
self._require(self.winner_share_bps.value + self.dispute_sink_share_bps.value <= UInt64(BPS_DENOMINATOR))
635635
self._assert_solvency()
636636
if st == UInt64(STATUS_CANCELLED) or st == UInt64(STATUS_DISPUTED):
637+
# DESIGN NOTE — why STATUS_DISPUTED is included here:
638+
#
639+
# Every exit path out of STATUS_DISPUTED (finalize_dispute,
640+
# cancel_dispute_and_market) ultimately either resolves to a
641+
# winner (STATUS_RESOLVED) or falls back to a full refund
642+
# (STATUS_CANCELLED). The CANCELLED path calls refund(), which
643+
# pays each holder at cost-basis. If pool < total_cost_basis,
644+
# early refund callers drain the pool and later callers get
645+
# nothing — an underflow / unfair distribution.
646+
#
647+
# To prevent a market from ever reaching STATUS_DISPUTED when
648+
# it cannot honour the refund-at-basis guarantee, we gate entry
649+
# to STATUS_DISPUTED behind the same pool >= tcb check that
650+
# guards STATUS_CANCELLED. This means:
651+
#
652+
# • It is IMPOSSIBLE to enter STATUS_DISPUTED when
653+
# pool < total_outstanding_cost_basis.
654+
# • The market can still be finalised via propose_resolution /
655+
# finalize_resolution — resolution is never blocked.
656+
# • The "stuck-in-dispute" liveness failure described in
657+
# security report #13 cannot occur because entry is gated.
658+
#
659+
# Known limitation: if heavy profitable trading drives
660+
# pool < total_cost_basis, the challenge mechanism becomes
661+
# unavailable. The correct long-term fix is a pro-rata fallback
662+
# refund mechanism (see issue #16). Until that is implemented,
663+
# this invariant is the safest conservative choice.
637664
self._require(self.pool_balance.value >= self.total_outstanding_cost_basis.value)
638665
if st != UInt64(STATUS_RESOLVED):
639666
self._assert_price_sum()

0 commit comments

Comments
 (0)