fix: guard init against reentrant call via self-recursive governance (#566)

[Aj-Kayvee]

Summary

A malicious governance contract could reenter init mid-way through the cross-contract validate_governance call. Because DataKey::Admin is written only after external calls complete, the reentrant invocation would pass the already-initialized guard and corrupt storage.

Problem

init (admin.rs) follows this order:

1. Check DataKey::Admin does not exist → panic if it does
2. validate_governance → external cross-contract call to governance
3. Write DataKey::Admin

A self-recursive governance contract could call init back during step 2. Since DataKey::Admin hasn't been written yet, the reentrant call passes the guard and could overwrite admin/governance state.

Solution

Introduce a contract-level init-in-progress marker (DataKey::Initializing):

• Set before the first external call (validate_governance)
• Checked at the top of init alongside the existing DataKey::Admin guard
• Removed once init completes

This provides defence-in-depth independent of the Soroban host's built-in reentry protection (Contract re-entry is not allowed).

Files changed

File	Change
settlement_contract/src/types.rs	Add Initializing variant to DataKey
settlement_contract/src/admin.rs	Set/check/remove Initializing marker in init
settlement_contract/src/tests/mod.rs	Declare reentrant_governance test module
settlement_contract/src/tests/reentrant_governance.rs	New — malicious governance mock for reentrancy test
settlement_contract/src/tests/admin_tests.rs	Two new tests + fix upstream 5-tuple bug
settlement_contract/src/payments.rs	Fix upstream unclosed proptest! block

Tests

rejects_reentrant_init_via_self_recursive_governance

Deploys a ReentrantGovernance contract whose get_fee_config reenters init on the settlement contract. Verifies the call is rejected (host reentry guard or init-in-progress marker).

rejects_init_while_initializing_marker_is_set

Directly sets DataKey::Initializing in the settlement contract's instance storage, then calls init. Verifies rejection with AlreadyInitialized (#1).

// Simulate an in-progress init by setting the Initializing marker.
env.as_contract(&contract_id, || {
    env.storage().instance().set(&DataKey::Initializing, &());
});
// init must reject because the Initializing marker is present.
client.init(&admins, &1, &governance, &recovery_address);
// → panics with AlreadyInitialized (#1)

Verification

cargo test --workspace
# 90 passed; 0 failed

Upstream fixes included

Two pre-existing issues on upstream main were fixed to restore compilation:

• Unclosed proptest! block in payments.rs:90
• 5-tuple destructure of 4-tuple setup() return in admin_tests.rs:363

Closes #566

[drips-wave]

@Aj-Kayvee Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits