Skip to content

Commit 29429ed

Browse files
Merge pull request #205 from khanavi272-spec/audit/yield-hook-reentrancy-190
audit: document reentrancy risks for external yield hooks (#190)
2 parents a34b66f + 1f680c7 commit 29429ed

1 file changed

Lines changed: 38 additions & 0 deletions

File tree

  • contracts/substream_contracts/src

contracts/substream_contracts/src/lib.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,44 @@ use soroban_sdk::{contract, contractevent, contractimpl, contracttype, vec, Addr
2525
/// group channel must contain exactly 5 creators -> Group must have exactly 5 creators.
2626
/// percentages must sum to 100 -> Percentages must sum to 100.
2727
28+
/// --- Reentrancy & External Call Security Audit (Issue #190) ---
29+
///
30+
/// This contract performs external calls via:
31+
/// - `TokenClient::transfer(...)`
32+
/// - Potential DEX/router interactions (buyback hooks)
33+
/// - Future yield routing integrations (via `YieldConfig`)
34+
///
35+
/// ## Risk: Cross-Contract Reentrancy
36+
///
37+
/// External contract calls may:
38+
/// - Re-enter this contract before state updates complete
39+
/// - Manipulate balances or subscription state
40+
///
41+
/// ## Mitigation Strategy
42+
///
43+
/// 1. **Checks-Effects-Interactions Pattern**
44+
/// - Internal state should be updated BEFORE external calls
45+
///
46+
/// 2. **Reentrancy Guard**
47+
/// - `ReentrancyGuard` is used to prevent nested execution
48+
/// - Critical functions should wrap execution with:
49+
/// `reentrancy_guard!(env, "function_name")`
50+
///
51+
/// 3. **Trusted Contract Assumptions**
52+
/// - `TokenClient` assumes compliant token contracts
53+
/// - External router (DEX) must be trusted or audited
54+
///
55+
/// 4. **Yield Hook Risk (Future)**
56+
/// - Any `YieldConfig` integration introduces untrusted execution paths
57+
/// - Must validate:
58+
/// - No state mutation after external call
59+
/// - Strict input/output validation
60+
///
61+
/// ## Auditor Notes
62+
///
63+
/// - No unsafe reentrancy pattern detected in current implementation
64+
/// - However, future yield hooks MUST enforce guard + ordering
65+
2866
// --- Issue #136: Subscription struct bitmask flags ---
2967
/// Bitmask flag: set when the free-trial-to-paid conversion event has been emitted.
3068
/// Bit 0 of `Subscription::flags`.

0 commit comments

Comments
 (0)