[SEC] Implement Reentrancy Guard for Payout Execution

[Queenode]

🔴 Priority: Critical
Difficulty: Hard
Estimated Effort: 2-3 days
Relevant Files: contracts/src/lib.rs, contracts/src/test.rs
Labels: security, priority:critical, soroban

Requirements

1. Reentrancy Vulnerability Remediation

   • The current payout() function updates state (HasReceivedPayout) after executing the token transfer:

     token_client.transfer(&env.current_contract_address(), &recipient, &pool_size);
     env.storage().persistent().set(&DataKey::HasReceivedPayout(recipient.clone()), &true);

   • While Soroban currently limits reentrancy via standard cross-contract calls, future protocol upgrades or specific token implementations (like custom wrapped tokens) might introduce reentrancy vectors.
   • Refactor payout() to strictly follow the Checks-Effects-Interactions (CEI) pattern.

2. State Update Relocation

   • Move the set(&DataKey::HasReceivedPayout, &true) state update to happen before token_client.transfer().
   • Ensure that the TTL extensions also occur before the external token transfer call.

3. Reentrancy Guard Mutex (Optional but Recommended)

   • Implement a simple lock in instance storage: DataKey::IsExecutingPayout.
   • Set it to true at the start of payout(), and clear it at the end.
   • Assert !IsExecutingPayout at the beginning of both contribute() and payout().

4. Testing

   • Unit test: Verify the CEI pattern is implemented (the state changes even if the transfer fails in a mock).
   • Write a mock malicious token contract that attempts to call payout() again during the transfer() callback.
   • Verify that the malicious reentrant call panics with "Already received payout" or "Reentrancy detected".
   • Target: 100% coverage on the payout function.

---

[legend-esc]

I'd like to work on this issue because payout execution is the contract's most security-sensitive operation, and protecting it against reentrancy is essential for preserving funds and maintaining correct protocol state. Even if Soroban's execution model reduces many traditional reentrancy risks compared to other smart contract platforms, explicitly guarding state-changing entry points is a good defensive practice, particularly when cross-contract token transfers are involved.

My approach would be to audit the complete payout flow to identify every external call and ensure all critical state transitions occur in a safe order. I'd introduce a contract-level reentrancy guard using a dedicated storage flag that is set immediately before payout execution begins and cleared only after the operation completes successfully. The implementation would ensure the guard is always released correctly, including in failure paths where the transaction aborts.

I'd also review the payout implementation using the checks-effects-interactions pattern wherever applicable, updating internal state before making external token transfer calls so the contract cannot be observed in an inconsistent intermediate state. If there are other entry points that can indirectly invoke payout or share the same critical state, I'd evaluate whether they should also participate in the same guard to avoid alternative execution paths.

To validate the implementation, I'd add unit tests covering normal payout execution, attempted reentrant invocation during payout, and verification that the guard resets correctly after both successful and failed transactions. Finally, I'd run the complete contract test suite to ensure the protection integrates cleanly without changing the intended payout behavior or introducing regressions.

[AbolareRoheemah]

Hello. I’m a smart contract developer with years of experience writing Rust, Cairo and Solidity. I'll be glad to work on this.🤝

[grantfox-oss]

🦊 GrantFox — @AbolareRoheemah has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #18)
2. Your PR will be reviewed by the Kolo-Org maintainers

Good luck! Track your progress on GrantFox.

[grantfox-oss]

🎉 This issue has been marked as completed on GrantFox as part of the Third Campaign campaign!

@AbolareRoheemah's PR #37 was approved and merged by @Queenode.

🏆 @AbolareRoheemah: You earned 35 FoxPoints for this contribution! Your current tier: Explorer (188 total points). Track your full progress on GrantFox.

👏 Great work, @AbolareRoheemah! Keep contributing to Kolo-Org.