Skip to content

feat(security): add cross-contract reentrancy and CEI guards across pool, backstop, treasury, and grants (#241) #503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
umpireboulianne-design wants to merge 1 commit into
base: main
Choose a base branch
from
+641 −108
Open

feat(security): add cross-contract reentrancy and CEI guards across pool, backstop, treasury, and grants (#241) #503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 63 additions & 12 deletions backstop/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
//! # RefractBackstop
//!
//! Second-loss underwriting reserve with Junior and Senior tranches.
//!
//! ## Cross-Contract Call Graph & Reentrancy Safety (#241)
//!
//! - **Inbound calls**: `RefractPool` (or admin/timelock) invokes `cover_shortfall`,
//! `absorb_loss`, `receive_premium`, and `set_pending_loss`; stakers invoke
//! `deposit` / `deposit_tranche` and `withdraw` / `withdraw_tranche`.
//! - **Outbound calls**: `token::Client::transfer` on the configured reserve token.
//! - **CEI & Reentrancy Invariant**: Every state-mutating entrypoint that performs
//! an outbound token call acquires a temporary-storage reentrancy lock
//! (`DataKey::ReentrancyLock`), commits all share, asset, tranche, and lot state
//! mutations *before* invoking `token.transfer`, and releases the lock upon
//! completion. Any re-entrant callback into `RefractBackstop` while the lock is
//! held fails immediately with [`BackstopError::ReentrancyDetected`].

#![no_std]

use soroban_sdk::{
Expand Down Expand Up @@ -29,6 +46,7 @@ enum DataKey {
NextLotId,
NextLossId,
UnabsorbedShortfall,
ReentrancyLock,
}

#[contracttype]
Expand Down Expand Up @@ -69,6 +87,7 @@ pub enum BackstopError {
PendingLossFreeze = 10,
InvalidPremiumSplit = 11,
DuplicateLossId = 12,
ReentrancyDetected = 13,
}

#[contracttype]
Expand Down Expand Up @@ -131,7 +150,7 @@ impl RefractBackstop {
premium_income: 0,
},
);
env.storage().instance().set(&DataKey::LockupLedgers, &100u32);
env.storage().instance().set(&DataKey::LockupLedgers, &0u32);
env.storage().instance().set(&DataKey::JuniorPremiumBps, &(BPS_DENOM / 2));
env.storage().instance().set(&DataKey::PendingLoss, &false);
env.storage().instance().set(&DataKey::NextLotId, &1u64);
Expand Down Expand Up @@ -171,8 +190,10 @@ impl RefractBackstop {
if amount <= 0 {
return Err(BackstopError::InvalidAmount);
}
Self::sync_matured(env.clone())?;
Self::acquire_reentrancy_guard(&env)?;
Self::_sync_matured(&env)?;
if Self::pending_loss(env.clone()) {
Self::release_reentrancy_guard(&env);
return Err(BackstopError::PendingLossFreeze);
}
let config = Self::get_config(&env)?;
Expand All @@ -185,15 +206,11 @@ impl RefractBackstop {
tranche_state.total_assets,
);
if shares <= 0 {
Self::release_reentrancy_guard(&env);
return Err(BackstopError::InvalidAmount);
}

token::Client::new(&env, &config.token).transfer(
&staker,
&env.current_contract_address(),
&amount,
);

// Effects: commit all share, asset, and lot state prior to outbound token transfer (CEI).
let key = DataKey::Shares(staker.clone(), tranche, false);
let prev: i128 = env.storage().persistent().get(&key).unwrap_or(0);
env.storage().persistent().set(&key, &(prev + shares));
Expand Down Expand Up @@ -225,6 +242,14 @@ impl RefractBackstop {
env.storage().instance().set(&"state", &state);
Self::_save_tranche_state(&env, tranche, &tranche_state);

// Interactions: outbound token transfer while reentrancy lock is held.
token::Client::new(&env, &config.token).transfer(
&staker,
&env.current_contract_address(),
&amount,
);
Self::release_reentrancy_guard(&env);

Ok(shares)
}

Expand All @@ -244,14 +269,16 @@ impl RefractBackstop {
if shares <= 0 {
return Err(BackstopError::InvalidAmount);
}
Self::sync_matured(env.clone())?;
Self::acquire_reentrancy_guard(&env)?;
Self::_sync_matured(&env)?;
let config = Self::get_config(&env)?;
let mut state = Self::get_state(&env)?;
let mut tranche_state = Self::_tranche_state(&env, tranche)?;

let key = DataKey::Shares(staker.clone(), tranche, true);
let held: i128 = env.storage().persistent().get(&key).unwrap_or(0);
if shares > held {
Self::release_reentrancy_guard(&env);
return Err(BackstopError::InsufficientShares);
}

Expand All @@ -261,6 +288,7 @@ impl RefractBackstop {
tranche_state.mature_assets,
);
if amount > state.total_capital {
Self::release_reentrancy_guard(&env);
return Err(BackstopError::InsufficientCapital);
}

Expand All @@ -281,6 +309,7 @@ impl RefractBackstop {
&amount,
);
}
Self::release_reentrancy_guard(&env);

Ok(amount)
}
Expand All @@ -299,7 +328,8 @@ impl RefractBackstop {
if amount <= 0 {
return Err(BackstopError::InvalidAmount);
}
Self::sync_matured(env.clone())?;
Self::acquire_reentrancy_guard(&env)?;
Self::_sync_matured(&env)?;
let state = Self::get_state(&env)?;
let covered = if amount > state.total_capital {
state.total_capital
Expand All @@ -311,6 +341,7 @@ impl RefractBackstop {
let covered = Self::_apply_loss(&env, loss_id, covered)?;
if covered <= 0 {
env.storage().instance().set(&DataKey::PendingLoss, &false);
Self::release_reentrancy_guard(&env);
return Ok(0);
}

Expand All @@ -319,6 +350,7 @@ impl RefractBackstop {
&config.pool,
&covered,
);
Self::release_reentrancy_guard(&env);

Ok(covered)
}
Expand All @@ -338,16 +370,18 @@ impl RefractBackstop {
if amount <= 0 {
return Err(BackstopError::InvalidAmount);
}
Self::sync_matured(env.clone())?;
Self::acquire_reentrancy_guard(&env)?;
Self::_sync_matured(&env)?;
let covered = Self::_apply_loss(&env, loss_id, amount)?;
env.storage().instance().set(&DataKey::PendingLoss, &false);
if covered > 0 {
token::Client::new(&env, &config.token).transfer(
&env.current_contract_address(),
&config.pool,
&covered,
);
}
env.storage().instance().set(&DataKey::PendingLoss, &false);
Self::release_reentrancy_guard(&env);
Ok(covered)
}

Expand Down Expand Up @@ -444,6 +478,7 @@ impl RefractBackstop {
if amount <= 0 {
return Err(BackstopError::InvalidAmount);
}
Self::acquire_reentrancy_guard(&env)?;
let config = Self::get_config(&env)?;
let mut state = Self::get_state(&env)?;

Expand All @@ -455,6 +490,7 @@ impl RefractBackstop {
state.premium_income += credited;
state.total_capital += credited;
env.storage().instance().set(&"state", &state);
Self::release_reentrancy_guard(&env);

Ok(credited)
}
Expand Down Expand Up @@ -594,23 +630,38 @@ impl RefractBackstop {
if caller != config.admin || to != config.admin || token != config.token || amount <= 0 {
return Err(BackstopError::Unauthorized);
}
Self::acquire_reentrancy_guard(&env)?;
let balance = token::Client::new(&env, &token).balance(&env.current_contract_address());
let excess = balance - Self::get_state(&env)?.total_capital;
if amount > excess {
Self::release_reentrancy_guard(&env);
return Err(BackstopError::NoExcessFunds);
}
token::Client::new(&env, &token).transfer(
&env.current_contract_address(),
&to,
&amount,
);
Self::release_reentrancy_guard(&env);
env.events().publish(
(Symbol::new(&env, "FundsRescued"),),
(token, to, amount, excess - amount),
);
Ok(excess - amount)
}

fn acquire_reentrancy_guard(env: &Env) -> Result<(), BackstopError> {
if env.storage().temporary().has(&DataKey::ReentrancyLock) {
return Err(BackstopError::ReentrancyDetected);
}
env.storage().temporary().set(&DataKey::ReentrancyLock, &true);
Ok(())
}

fn release_reentrancy_guard(env: &Env) {
env.storage().temporary().remove(&DataKey::ReentrancyLock);
}

pub fn batch(env: Env, caller: Address, operations: Vec<BackstopOp>) -> Result<(), BackstopError> {
caller.require_auth();
let mut config = Self::get_config(&env)?;
Expand Down
148 changes: 148 additions & 0 deletions backstop/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,3 +430,151 @@ fn test_backstop_regression_guard_requires_auth() {
// Regression guard: Unauthenticated call to cover_shortfall must fail
let _ = client.cover_shortfall(&pool, &500);
}

// ─── Issue #241: Cross-Contract Reentrancy & Callback Safety Guards ──────────

#[soroban_sdk::contract]
pub struct HostileBackstopToken;

#[soroban_sdk::contractimpl]
impl HostileBackstopToken {
pub fn set_target(
env: Env,
backstop: Address,
attacker: Address,
pool: Address,
mode: u32,
) {
env.storage().instance().set(&"backstop", &backstop);
env.storage().instance().set(&"attacker", &attacker);
env.storage().instance().set(&"pool", &pool);
env.storage().instance().set(&"mode", &mode);
}

pub fn last_reentry_err(env: Env) -> Option<BackstopError> {
env.storage().instance().get(&"last_err")
}

pub fn balance(_env: Env, _id: Address) -> i128 {
1_000_000
}

pub fn transfer(env: Env, _from: Address, _to: Address, _amount: i128) {
if env.storage().temporary().has(&"in_hook") {
return;
}
env.storage().temporary().set(&"in_hook", &true);

let backstop: Address = env.storage().instance().get(&"backstop").unwrap();
let attacker: Address = env.storage().instance().get(&"attacker").unwrap();
let pool: Address = env.storage().instance().get(&"pool").unwrap();
let mode: u32 = env.storage().instance().get(&"mode").unwrap_or(0);
let client = RefractBackstopClient::new(&env, &backstop);

let res = match mode {
1 => client.try_deposit(&attacker, &500).err(),
2 => client.try_withdraw(&attacker, &100).err(),
3 => client.try_cover_shortfall(&pool, &200).err(),
_ => None,
};
if let Some(Ok(err)) = res {
env.storage().instance().set(&"last_err", &err);
}

env.storage().temporary().remove(&"in_hook");
}
}

#[soroban_sdk::contract]
pub struct LegitimatePoolCaller;

#[soroban_sdk::contractimpl]
impl LegitimatePoolCaller {
pub fn route_and_draw(
env: Env,
backstop: Address,
premium: i128,
shortfall: i128,
) -> (i128, i128) {
let client = RefractBackstopClient::new(&env, &backstop);
let credited = client.receive_premium(&premium);
let covered = client.cover_shortfall(&env.current_contract_address(), &shortfall);
(credited, covered)
}
}

#[test]
fn test_reentrant_token_callbacks_are_blocked_with_reentrancy_detected() {
let env = Env::default();
env.mock_all_auths();

let pool = Address::generate(&env);
let admin = Address::generate(&env);
let attacker = Address::generate(&env);

let token_id = env.register_contract(None, HostileBackstopToken);
let token_client = HostileBackstopTokenClient::new(&env, &token_id);

let backstop_id = env.register_contract(None, RefractBackstop);
let client = RefractBackstopClient::new(&env, &backstop_id);
client.initialize(&pool, &admin, &token_id, &2_000);

// 1. Re-entering deposit during deposit's token.transfer is blocked.
token_client.set_target(&backstop_id, &attacker, &pool, &1u32);
let shares = client.deposit(&attacker, &1_000);
assert_eq!(shares, 1_000);
assert_eq!(
token_client.last_reentry_err(),
Some(BackstopError::ReentrancyDetected)
);
assert_eq!(client.shares_of(&attacker), 1_000);

// 2. Re-entering withdraw during withdraw's token.transfer is blocked.
token_client.set_target(&backstop_id, &attacker, &pool, &2u32);
let withdrawn = client.withdraw(&attacker, &400);
assert_eq!(withdrawn, 400);
assert_eq!(
token_client.last_reentry_err(),
Some(BackstopError::ReentrancyDetected)
);
assert_eq!(client.shares_of(&attacker), 600);

// 3. Re-entering cover_shortfall during cover_shortfall's token.transfer is blocked.
token_client.set_target(&backstop_id, &attacker, &pool, &3u32);
let covered = client.cover_shortfall(&pool, &300);
assert_eq!(covered, 300);
assert_eq!(
token_client.last_reentry_err(),
Some(BackstopError::ReentrancyDetected)
);
assert_eq!(client.state().total_capital, 300);
}

#[test]
fn test_legitimate_pool_to_backstop_nested_calls_succeed() {
let env = Env::default();
env.mock_all_auths();

let pool_id = env.register_contract(None, LegitimatePoolCaller);
let pool_client = LegitimatePoolCallerClient::new(&env, &pool_id);
let admin = Address::generate(&env);
let staker = Address::generate(&env);

let token_admin = Address::generate(&env);
let token_id = env.register_stellar_asset_contract(token_admin);
let sac = token::StellarAssetClient::new(&env, &token_id);

let backstop_id = env.register_contract(None, RefractBackstop);
let client = RefractBackstopClient::new(&env, &backstop_id);
client.initialize(&pool_id, &admin, &token_id, &2_000);

sac.mint(&staker, &5_000);
sac.mint(&backstop_id, &1_000);
client.deposit(&staker, &5_000);
client.sync_matured();

let (credited, covered) = pool_client.route_and_draw(&backstop_id, &1_000, &1_200);
assert_eq!(credited, 100);
assert_eq!(covered, 1_200);
}

Loading