Skip to content

Commit 9dcc024

Browse files
authored
fix: low-severity defensive hardening (vault + factory)
* fix: defensive index removal and cap denied-issuer list * fix: controlled error on missing VaultMeta instead of panic * fix: clear freed index slot when tail is missing to avoid ghost entry
1 parent fc9f26d commit 9dcc024

5 files changed

Lines changed: 37 additions & 10 deletions

File tree

contracts/vc-vault-factory/src/errors.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,6 @@ pub enum FactoryError {
1818
FeeNotConfigured = 5,
1919
/// Custom fee expiry timestamp is not in the future.
2020
ExpiryInPast = 6,
21+
/// VaultMeta is missing (constructor never ran or state lost).
22+
NotInitialized = 7,
2123
}

contracts/vc-vault-factory/src/storage.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use soroban_sdk::{contracttype, Address, BytesN, Env, Symbol};
1+
use soroban_sdk::{contracttype, panic_with_error, Address, BytesN, Env, Symbol};
2+
3+
use crate::errors::FactoryError;
24

35
const ONE_DAY_LEDGERS: u32 = 17_280; // ~5s per ledger
46

@@ -60,7 +62,7 @@ pub fn get_vault_init_meta(e: &Env) -> VaultInitMeta {
6062
e.storage()
6163
.instance()
6264
.get::<Symbol, VaultInitMeta>(&Symbol::new(e, "VaultMeta"))
63-
.unwrap()
65+
.unwrap_or_else(|| panic_with_error!(e, FactoryError::NotInitialized))
6466
}
6567

6668
pub fn set_deployed(e: &Env, vault_address: &Address) {

contracts/vc-vault/src/constants.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,6 @@ pub const MAX_DID_URI_LEN: u32 = 256;
4949
pub const MAX_ISSUER_DID_LEN: u32 = 256;
5050
/// Maximum bytes for revocation `date` strings (ISO 8601).
5151
pub const MAX_DATE_LEN: u32 = 64;
52+
/// Maximum number of issuers a vault may hold in its denied (block) list.
53+
/// Bounds storage growth; the list only grows via admin `deny_issuer` calls.
54+
pub const MAX_DENIED_ISSUERS: u32 = 1_000;

contracts/vc-vault/src/storage/credential.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,17 @@ pub fn remove_vc_from_index(e: &Env, vc_id: &String) {
125125
}
126126
let last = count - 1;
127127
if position != last {
128-
let last_id = read_vc_id_at(e, last).unwrap();
129-
write_vc_id_at(e, position, &last_id);
130-
write_vc_position(e, &last_id, position);
128+
// Normal swap-and-pop moves the last entry into the freed slot.
129+
// Defensive: if the last slot is somehow missing (corrupt state), clear
130+
// the freed slot instead of leaving the removed id there — otherwise it
131+
// becomes a ghost index entry. Never panic on the missing slot.
132+
match read_vc_id_at(e, last) {
133+
Some(last_id) => {
134+
write_vc_id_at(e, position, &last_id);
135+
write_vc_position(e, &last_id, position);
136+
}
137+
None => remove_vc_id_at(e, position),
138+
}
131139
}
132140
remove_vc_id_at(e, last);
133141
remove_vc_position(e, vc_id);

contracts/vc-vault/src/storage/issuer.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
//! Denied issuer index storage. O(1) operations via swap-and-pop.
22
3-
use crate::constants::{PERSISTENT_TTL_EXTEND_TO, PERSISTENT_TTL_THRESHOLD};
3+
use crate::constants::{MAX_DENIED_ISSUERS, PERSISTENT_TTL_EXTEND_TO, PERSISTENT_TTL_THRESHOLD};
4+
use crate::error::ContractError;
45
use super::VcVaultDataKey;
5-
use soroban_sdk::{Address, Env};
6+
use soroban_sdk::{panic_with_error, Address, Env};
67

78
// --- Denied issuer index ---
89

@@ -84,6 +85,9 @@ pub fn append_denied_issuer_to_index(e: &Env, issuer: &Address) {
8485
return;
8586
}
8687
let count = read_denied_issuer_count(e);
88+
if count >= MAX_DENIED_ISSUERS {
89+
panic_with_error!(e, ContractError::IssuerListTooLong);
90+
}
8791
write_denied_issuer_at(e, count, issuer);
8892
write_denied_issuer_position(e, issuer, count);
8993
write_denied_issuer_count(e, count + 1);
@@ -101,9 +105,17 @@ pub fn remove_denied_issuer_from_index(e: &Env, issuer: &Address) {
101105
}
102106
let last = count - 1;
103107
if position != last {
104-
let last_addr = read_denied_issuer_at(e, last).unwrap();
105-
write_denied_issuer_at(e, position, &last_addr);
106-
write_denied_issuer_position(e, &last_addr, position);
108+
// Normal swap-and-pop moves the last entry into the freed slot.
109+
// Defensive: if the last slot is somehow missing (corrupt state), clear
110+
// the freed slot instead of leaving the removed issuer there — otherwise
111+
// it becomes a ghost index entry. Never panic on the missing slot.
112+
match read_denied_issuer_at(e, last) {
113+
Some(last_addr) => {
114+
write_denied_issuer_at(e, position, &last_addr);
115+
write_denied_issuer_position(e, &last_addr, position);
116+
}
117+
None => remove_denied_issuer_at(e, position),
118+
}
107119
}
108120
remove_denied_issuer_at(e, last);
109121
remove_denied_issuer_position(e, issuer);

0 commit comments

Comments
 (0)