Problem
referral_storage::set_tier_config (contracts/referral_storage/src/lib.rs:237-266) writes the caller-supplied TierConfig to persistent storage before validating it, then validates it, then writes the identical value to the same key a second time:
pub fn set_tier_config(env: Env, admin: Address, tier: u32, config: TierConfig) {
admin.require_auth();
// ... admin check ...
if tier > 2 {
panic_with_error!(&env, Error::InvalidTier);
}
let tier_key = ReferralKey::TierConfig(tier);
env.storage().persistent().set(&tier_key, &config); // <-- write #1, unvalidated
env.storage().persistent().extend_ttl(&tier_key, MIN_BUMP_THRESHOLD, PERSISTENT_BUMP_TARGET);
// Validate config parameters
let discount_bps = ((config.total_rebate_bps as u64) * (config.discount_share_bps as u64) / 10000) as u32;
let rebate_bps = if config.total_rebate_bps >= discount_bps {
config.total_rebate_bps - discount_bps
} else {
panic_with_error!(&env, Error::InvalidTierConfig);
};
if discount_bps > 10000 || rebate_bps > 10000 || config.total_rebate_bps > 10000 || config.discount_share_bps > 10000 {
panic_with_error!(&env, Error::InvalidTierConfig);
}
env.storage()
.persistent()
.set(&ReferralKey::TierConfig(tier), &config); // <-- write #2, redundant
}Because Soroban transactions are atomic, the early unvalidated write never actually persists on the panic path (the whole call reverts), so this isn't independently exploitable. But it is a genuine defect: every successful call performs two identical persistent writes (and two extend_ttl calls implied by the surrounding code) instead of one, wasting storage-write resource budget on every admin config update, and the validation-after-mutation ordering inverts the checks-then-act pattern used everywhere else in this file (set_referrer_tier, set_tier_upgrade_threshold, etc. all validate before writing).
Why it matters
Beyond the wasted resources, the ordering is confusing to read and easy to regress: a future edit to the validation block could accidentally rely on config already being in storage, or a future refactor could drop the second write and silently leave the invalid-looking-but-actually-fine first write as the only one, without anyone noticing since both paths currently produce the same end state.
Suggested fix
Move the entire validation block (the discount_bps/rebate_bps computation and both bound checks) above the first env.storage().persistent().set(...) call, and delete the now-redundant second write and its preceding extend_ttl. This makes set_tier_config validate-then-write like its siblings, with a single write per call.
Problem
referral_storage::set_tier_config(contracts/referral_storage/src/lib.rs:237-266) writes the caller-suppliedTierConfigto persistent storage before validating it, then validates it, then writes the identical value to the same key a second time:Because Soroban transactions are atomic, the early unvalidated write never actually persists on the panic path (the whole call reverts), so this isn't independently exploitable. But it is a genuine defect: every successful call performs two identical persistent writes (and two
extend_ttlcalls implied by the surrounding code) instead of one, wasting storage-write resource budget on every admin config update, and the validation-after-mutation ordering inverts the checks-then-act pattern used everywhere else in this file (set_referrer_tier,set_tier_upgrade_threshold, etc. all validate before writing).Why it matters
Beyond the wasted resources, the ordering is confusing to read and easy to regress: a future edit to the validation block could accidentally rely on
configalready being in storage, or a future refactor could drop the second write and silently leave the invalid-looking-but-actually-fine first write as the only one, without anyone noticing since both paths currently produce the same end state.Suggested fix
Move the entire validation block (the
discount_bps/rebate_bpscomputation and both bound checks) above the firstenv.storage().persistent().set(...)call, and delete the now-redundant second write and its precedingextend_ttl. This makesset_tier_configvalidate-then-write like its siblings, with a single write per call.