Root + Impact
Description
Normal Behavior
Users should be able to update their stored secrets multiple times, allowing them to change compromised secrets, fix mistakes, or rotate credentials as needed.
Issue
The contract suffers from a critical resource collision vulnerability that prevents users from updating their secrets after the initial storage. The set_secret
function uses move_to
which can only be called once per resource type per address. Any subsequent calls to update a secret will fail with RESOURCE_ALREADY_EXISTS
error, making all secrets permanently immutable.
public entry fun set_secret(caller: &signer, secret: vector<u8>) {
let secret_vault = Vault{secret: string::utf8(secret)};
move_to(caller, secret_vault);
event::emit(SetNewSecret {});
}
Risk
Likelihood:
Every user who calls set_secret
twice will encounter this error
The limitation applies to all users without exception
No alternative update mechanism exists in the contract
Users will immediately discover this limitation in normal usage
Impact:
Complete inability to update secrets after initial storage
Permanent exposure risk if secrets are compromised
Users cannot fix accidentally stored incorrect secrets
Violation of basic security practices requiring credential rotation
Contract becomes single-use only, severely limiting functionality
Business requirements for secret management cannot be met
Proof of Concept
The following tests demonstrate the resource collision vulnerability and its impact:
#[test(owner = @0xcc, user_a = @0x111, user_b = @0x222)]
fun test_resource_collision_vulnerability(owner: &signer, user_a: &signer, user_b: &signer) acquires Vault {
account::create_account_for_test(signer::address_of(owner));
account::create_account_for_test(signer::address_of(user_a));
account::create_account_for_test(signer::address_of(user_b));
set_secret(owner, b"owner_secret_v1");
set_secret(user_a, b"user_a_secret_v1");
set_secret(user_b, b"user_b_secret_v1");
}
#[test(user = @0x444)]
#[expected_failure]
fun test_cannot_update_secret_direct(user: &signer) {
account::create_account_for_test(signer::address_of(user));
set_secret(user, b"my_first_secret");
set_secret(user, b"my_updated_secret");
}
#[test(user = @0x555)]
fun test_what_proper_update_should_look_like(user: &signer) acquires Vault {
set_secret(user, b"initial_secret");
let user_addr = signer::address_of(user);
let vault_mut = borrow_global_mut<Vault>(user_addr);
vault_mut.secret = string::utf8(b"updated_secret");
}
Recommended Mitigation
Implement proper update functionality using borrow_global_mut
instead of always using move_to
:
+ public entry fun update_secret(caller: &signer, secret: vector<u8>) acquires Vault {
+ let caller_addr = signer::address_of(caller);
+
+ if (exists<Vault>(caller_addr)) {
+ // Update existing secret
+ let vault = borrow_global_mut<Vault>(caller_addr);
+ vault.secret = string::utf8(secret);
+ } else {
+ // Create new secret
+ let secret_vault = Vault{secret: string::utf8(secret)};
+ move_to(caller, secret_vault);
+ }
+
+ event::emit(SetNewSecret {});
+ }
- public entry fun set_secret(caller: &signer, secret: vector<u8>) {
+ public entry fun set_secret(caller: &signer, secret: vector<u8>) acquires Vault {
+ // Use the new update logic
+ update_secret(caller, secret);
- }