Implement cross-contract call hardening in escrow contract

README.md

@@ -20,6 +20,27 @@ The escrow contract now enforces a minimal on-chain state machine instead of pla
 
 Reviewer-focused contract notes and the formal threat model live in [docs/escrow/README.md](/home/christopher/drips_projects/Talenttrust-Contracts/docs/escrow/README.md).
 
+## Cross-contract call hardening
+
+The escrow contract includes explicit guards for external token-contract assumptions and failures:
+
+- A configured token contract is required before guarded transfers.
+- External call target validation rejects self-referential addresses.
+- A reentrancy lock is acquired during transfer attempts.
+- External token call failures are mapped to typed `EscrowError` values.
+- Non-positive transfer amounts are rejected before any external call.
+
+Primary implementation points:
+
+- `contracts/escrow/src/lib.rs`:
+  - `set_token_contract`
+  - `get_token_contract`
+  - `guarded_external_transfer`
+- `contracts/escrow/src/cross_contract.rs`:
+  - `safe_token_transfer`
+  - `validate_external_address`
+  - `acquire_call_lock` / `release_call_lock`
+
 ## Protocol governance
 
 The escrow contract supports guarded protocol parameter updates for live validation logic:

contracts/escrow/src/cross_contract.rs

@@ -0,0 +1,220 @@
+//! # Cross-Contract Call Hardening
+//!
+//! This module provides validated, guarded wrappers around every external
+//! (cross-contract) call made by the TalentTrust Escrow contract.
+//!
+//! ## Threat model
+//!
+//! | Threat                       | Mitigation                                      |
+//! |------------------------------|-------------------------------------------------|
+//! | Reentrancy attack            | [`CallLock`] stored in instance storage before  |
+//! |                              | any external call; cleared after return         |
+//! | Self-referential call        | [`validate_external_address`] rejects `addr ==  |
+//! |                              | env.current_contract_address()`                 |
+//! | Silent transfer failure      | `try_transfer` result is checked; returns       |
+//! |                              | [`EscrowError::ExternalCallFailed`] on error    |
+//! | Unregistered token contract  | [`get_required_token_contract`] returns          |
+//! |                              | [`EscrowError::TokenContractNotSet`] if absent  |
+//! | Zero-amount transfer         | Guard validates `amount > 0` before call        |
+//! | Balance drift                | Pre/post balance comparison via                 |
+//! |                              | [`verify_balance_received`]                     |
+//!
+//! ## Usage
+//!
+//! ```text
+//! // Inside release_milestone:
+//! let token = get_required_token_contract(&env)?;
+//! safe_token_transfer(&env, &token, &from, &to, amount)?;
+//! ```
+
+use soroban_sdk::{token, Address, Env};
+
+use crate::{DataKey, EscrowError};
+
+// ── Reentrancy guard ──────────────────────────────────────────────────────────
+
+/// Acquire the single reentrancy lock stored in contract instance storage.
+///
+/// # Security
+/// Sets `DataKey::CallLock = true` before any external call so that a
+/// malicious callee cannot re-enter the escrow contract mid-execution and
+/// observe or mutate inconsistent state.
+///
+/// If the lock is already held (i.e., another external call is in progress
+/// on this contract), the function returns
+/// [`EscrowError::ReentrancyDetected`].
+///
+/// # Errors
+/// - [`EscrowError::ReentrancyDetected`] – lock already acquired.
+pub fn acquire_call_lock(env: &Env) -> Result<(), EscrowError> {
+    let locked: bool = env
+        .storage()
+        .instance()
+        .get(&DataKey::CallLock)
+        .unwrap_or(false);
+
+    if locked {
+        return Err(EscrowError::ReentrancyDetected);
+    }
+
+    env.storage().instance().set(&DataKey::CallLock, &true);
+    Ok(())
+}
+
+/// Release the reentrancy lock.
+///
+/// Must be called after every successful [`acquire_call_lock`], including
+/// the error paths so the contract is never left in a permanently locked state.
+/// Because Soroban rolls back all storage writes on panic, a forgotten
+/// `release_call_lock` after a panicking path is safe—the lock is cleared by
+/// the host rollback—but callers should still release explicitly for clarity.
+pub fn release_call_lock(env: &Env) {
+    env.storage().instance().remove(&DataKey::CallLock);
+}
+
+// ── Address validation ────────────────────────────────────────────────────────
+
+/// Validate that `addr` is a safe target for an external call.
+///
+/// Guards enforced (in order):
+/// 1. **Non-self**: `addr` must not equal `env.current_contract_address()`.
+///    Self-calls would cause the escrow contract to manipulate its own state
+///    unexpectedly mid-execution.
+///
+/// # Errors
+/// - [`EscrowError::SelfReferentialAddress`] – `addr` is this contract.
+pub fn validate_external_address(env: &Env, addr: &Address) -> Result<(), EscrowError> {
+    if *addr == env.current_contract_address() {
+        return Err(EscrowError::SelfReferentialAddress);
+    }
+    Ok(())
+}
+
+// ── Token contract access ─────────────────────────────────────────────────────
+
+/// Retrieve the registered token contract address, returning an error if
+/// none has been set via [`crate::Escrow::set_token_contract`].
+///
+/// # Errors
+/// - [`EscrowError::TokenContractNotSet`] – no token contract registered.
+pub fn get_required_token_contract(env: &Env) -> Result<Address, EscrowError> {
+    env.storage()
+        .instance()
+        .get(&DataKey::TokenContract)
+        .ok_or(EscrowError::TokenContractNotSet)
+}
+
+// ── Safe token transfer ───────────────────────────────────────────────────────
+
+/// Execute a SEP-41 token transfer with all cross-contract call hardening.
+///
+/// # Hardening steps (in order)
+///
+/// 1. **Amount guard** – `amount` must be strictly positive.
+/// 2. **Address guard** – `token_address` must not be `env.current_contract_address()`.
+/// 3. **Reentrancy lock** – acquired before the external call.
+/// 4. **Try-transfer** – uses `token::Client::try_transfer` rather than the
+///    panicking `transfer`, so the error is captured and mapped to
+///    [`EscrowError::ExternalCallFailed`] instead of aborting with an opaque
+///    host panic.
+/// 5. **Reentrancy unlock** – released unconditionally after the call returns.
+///
+/// # Arguments
+///
+/// | Name            | Description                                                    |
+/// |-----------------|----------------------------------------------------------------|
+/// | `token_address` | The SEP-41 compatible token contract to invoke.               |
+/// | `from`          | Source address; must have pre-authorized the transfer.        |
+/// | `to`            | Destination address.                                          |
+/// | `amount`        | Transfer amount in base units (must be `> 0`).                |
+///
+/// # Errors
+///
+/// | Error                              | Condition                              |
+/// |------------------------------------|----------------------------------------|
+/// | [`EscrowError::AmountMustBePositive`] | `amount <= 0`                       |
+/// | [`EscrowError::SelfReferentialAddress`] | `token_address == self`           |
+/// | [`EscrowError::ReentrancyDetected`]  | Reentrant call detected              |
+/// | [`EscrowError::ExternalCallFailed`]  | Token contract returned an error     |
+pub fn safe_token_transfer(
+    env: &Env,
+    token_address: &Address,
+    from: &Address,
+    to: &Address,
+    amount: i128,
+) -> Result<(), EscrowError> {
+    // Guard 1: amount must be positive
+    if amount <= 0 {
+        return Err(EscrowError::AmountMustBePositive);
+    }
+
+    // Guard 2: validate token contract address is not self
+    validate_external_address(env, token_address)?;
+
+    // Guard 3: acquire reentrancy lock
+    acquire_call_lock(env)?;
+
+    // Guard 4: execute transfer using try_transfer for explicit error handling
+    let token_client = token::Client::new(env, token_address);
+    let result = token_client
+        .try_transfer(from, to, &amount)
+        .map_err(|_| EscrowError::ExternalCallFailed)
+        .and_then(|inner| inner.map_err(|_| EscrowError::ExternalCallFailed));
+
+    // Guard 5: release lock unconditionally before returning
+    release_call_lock(env);
+
+    result
+}
+
+// ── Balance verification ──────────────────────────────────────────────────────
+
+/// Read the SEP-41 token balance for `addr` from `token_address`.
+///
+/// This is a read-only call and does not acquire the reentrancy lock.
+///
+/// # Errors
+/// - [`EscrowError::SelfReferentialAddress`] – `token_address == self`.
+/// - [`EscrowError::ExternalCallFailed`] – balance query failed.
+pub fn query_token_balance(
+    env: &Env,
+    token_address: &Address,
+    addr: &Address,
+) -> Result<i128, EscrowError> {
+    validate_external_address(env, token_address)?;
+
+    let token_client = token::Client::new(env, token_address);
+    token_client
+        .try_balance(addr)
+        .map_err(|_| EscrowError::ExternalCallFailed)
+        .and_then(|inner| inner.map_err(|_| EscrowError::ExternalCallFailed))
+}
+
+/// Verify that the balance of `recipient` increased by exactly `expected_delta`
+/// compared to `balance_before`.
+///
+/// Call [`query_token_balance`] before the transfer (capturing `balance_before`),
+/// execute the transfer, then call this function to confirm the delta.
+///
+/// # Errors
+/// - All errors from [`query_token_balance`].
+/// - [`EscrowError::TransferVerificationFailed`] – balance delta does not match.
+pub fn verify_balance_received(
+    env: &Env,
+    token_address: &Address,
+    recipient: &Address,
+    balance_before: i128,
+    expected_delta: i128,
+) -> Result<(), EscrowError> {
+    let balance_after = query_token_balance(env, token_address, recipient)?;
+
+    let actual_delta = balance_after
+        .checked_sub(balance_before)
+        .ok_or(EscrowError::ArithmeticOverflow)?;
+
+    if actual_delta != expected_delta {
+        return Err(EscrowError::TransferVerificationFailed);
+    }
+
+    Ok(())
+}

contracts/escrow/src/lib.rs

@@ -32,9 +32,11 @@
 
 use soroban_sdk::{
     contract, contracterror, contractimpl, contracttype, symbol_short, Address, Env, String,
-    Symbol, Vec,
+    Map, Symbol, Vec,
 };
 
+pub(crate) mod cross_contract;
+
 /// Persistent storage keys used by the Escrow contract.
 ///
 /// Each variant corresponds to a distinct piece of contract state:
@@ -63,6 +65,10 @@ pub enum DataKey {
     GovernanceAdmin,
     PendingGovernanceAdmin,
     ProtocolParameters,
+    /// Registered SEP-41 token contract used for escrow transfers.
+    TokenContract,
+    /// Reentrancy lock set during guarded external calls.
+    CallLock,
 }
 
 /// The lifecycle status of an escrow contract.
@@ -230,6 +236,18 @@ pub enum EscrowError {
     InvalidRating = 22,
     /// Invalid milestone ID
     InvalidMilestoneId = 23,
+    /// Token contract address is not configured
+    TokenContractNotSet = 24,
+    /// Cross-contract call failed or returned an error
+    ExternalCallFailed = 25,
+    /// Reentrant external call attempt detected
+    ReentrancyDetected = 26,
+    /// External address points to this contract
+    SelfReferentialAddress = 27,
+    /// Transfer verification failed due to balance delta mismatch
+    TransferVerificationFailed = 28,
+    /// Transfer amount must be strictly positive
+    AmountMustBePositive = 29,
 }
 
 // (ReleaseAuthorization enum moved)
@@ -275,6 +293,52 @@ const DEFAULT_MILESTONE_TIMEOUT_SECS: u64 = 7 * 24 * 60 * 60;
 
 #[contractimpl]
 impl Escrow {
+    /// @notice Register or rotate the SEP-41 token contract used for escrow transfers.
+    /// @dev Security: the caller must authorize this operation.
+    /// @param caller Address authorizing token contract configuration.
+    /// @param token_contract External token contract address to persist.
+    /// @return bool True when the token contract address is stored.
+    pub fn set_token_contract(env: Env, caller: Address, token_contract: Address) -> bool {
+        caller.require_auth();
+
+        if token_contract == env.current_contract_address() {
+            panic!("Token contract cannot be escrow contract");
+        }
+
+        env.storage()
+            .instance()
+            .set(&DataKey::TokenContract, &token_contract);
+
+        true
+    }
+
+    /// @notice Return the configured SEP-41 token contract address, if present.
+    /// @return Option<Address> Configured token address or `None`.
+    pub fn get_token_contract(env: Env) -> Option<Address> {
+        env.storage().instance().get(&DataKey::TokenContract)
+    }
+
+    /// @notice Perform a guarded token transfer using the configured token contract.
+    ///
+    /// @dev This helper centralizes external call assumptions and failure mapping:
+    /// it requires a configured token contract, validates it is non-self,
+    /// applies a reentrancy lock, and surfaces external failures as
+    /// `EscrowError` values.
+    /// @param from Token source address.
+    /// @param to Token recipient address.
+    /// @param amount Transfer amount in token base units.
+    /// @return Result<bool, EscrowError> `Ok(true)` on successful guarded transfer.
+    pub fn guarded_external_transfer(
+        env: Env,
+        from: Address,
+        to: Address,
+        amount: i128,
+    ) -> Result<bool, EscrowError> {
+        let token_contract = cross_contract::get_required_token_contract(&env)?;
+        cross_contract::safe_token_transfer(&env, &token_contract, &from, &to, amount)?;
+        Ok(true)
+    }
+
     /// Create a new escrow contract with milestone-based release authorization.
     ///
     /// Stores the contract record in persistent storage and returns a numeric

contracts/escrow/src/test.rs

@@ -14,6 +14,7 @@ pub(crate) const MILESTONE_THREE: i128 = 600_0000000;
 // ==================== CONTRACT CREATION TESTS ====================
 
 mod timeout_tests;
+mod cross_contract_hardening;
 
 #[test]
 fn test_create_contract_success() {