Secret Vault
First Flight #46
Beginner FriendlyWallet
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Secret Can Be Overwritten and Permanently Lost

devansh2020raulo

Root + Impact

Description

  • The set_secret function is intended to store a secret for a user by creating a Vault resource.

  • The function does not check if a Vault resource already exists at the caller's address. When a user calls set_secret more than once, the existing Vault is replaced, and the original secret is permanently destroyed.

// Root cause in the codebase with @> marks to highlight the relevant section
@>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:

  • Reason 1: A user calls set_secret multiple times, accidentally overwriting their secret.

  • The function's behavior is unintuitive, as users typically expect a "set" function to either update or fail, not silently destroy and replace.

Impact:

  • Permanent loss of a user's secret data.

  • Failure of the contract's core promise, which is to securely store a secret.

Proof of Concept

The following test demonstrates that a user can overwrite their existing Vault.

  1. A user calls set_secret for the first time with an initial secret.

  2. The test verifies the secret is stored correctly.

  3. The user then calls set_secret a second time with a new secret.

  4. The test confirms that the new secret has replaced the original one, proving the first Vault and its contents were destroyed and are now unrecoverable.

#[test(owner = @0xcc)]
fun test_secret_overwrite(owner: &signer) acquires Vault {
use aptos_framework::account;
// Set up test environment
account::create_account_for_test(signer::address_of(owner));
// Set the first secret
let secret1 = b"this is the original secret";
set_secret(owner, secret1);
// Verify the first secret is stored
let owner_address = signer::address_of(owner);
let vault1 = borrow_global<Vault>(owner_address);
assert!(vault1.secret == string::utf8(secret1), 1);
// Call set_secret again to overwrite the vault
let secret2 = b"this is the new secret";
set_secret(owner, secret2);
// Verify the first secret is gone and the second is stored
let vault2 = borrow_global<Vault>(owner_address);
assert!(vault2.secret == string::utf8(secret2), 2);
// The original secret is now unrecoverable
}

Recommended Mitigation

The recommended fix is to add a check to ensure a Vault does not already exist for the caller before creating a new one.

This is achieved by adding assert!(!exists<Vault>(signer::address_of(caller)), 2); at the beginning of the set_secret function. This line checks if a Vault resource is already stored under the caller's account. If one exists, the transaction will revert with an error, preventing the overwrite. This change enforces a "create-only" behavior, protecting existing secrets from being lost. To allow for updates, a separate, explicit function should be created.

- 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 {});
- }
+ public entry fun set_secret(caller:&signer,secret:vector<u8>){
+ assert!(!exists<Vault>(signer::address_of(caller)), 2); // Revert if a Vault already exists
+ let secret_vault = Vault{secret: string::utf8(secret)};
+ move_to(caller,secret_vault);
+ event::emit(SetNewSecret {});
+ }
Updates

Lead Judging Commences

bube Lead Judge 11 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.