Secret Vault on Aptos
First Flight #46
Beginner FriendlyWallet
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

SecretVault Audit Submission Report

varunpareek690

Executive Summary

The SecretVault Move smart contract is intended to allow users to securely store and retrieve private secrets.
However, our analysis revealed multiple security flaws that compromise confidentiality and proper access control.
These issues allow unauthorized access, overwrite stored data, and reduce auditability.

Finding 1: Unauthorized Secret Retrieval

Description

The get_secret function is intended to allow only the owner of a vault to retrieve their secret.
However, the function accepts an arbitrary address argument and compares it against a hardcoded @owner.
This means that anyone who knows the owner’s address can retrieve the secret, since the check is not tied to the transaction signer.

#[view]
public fun get_secret(caller: address): String acquires Vault {
assert!(caller == @owner, NOT_OWNER);
let vault = borrow_global<Vault>(@owner);
vault.secret
}

Risk

Likelihood: High

  • Anyone can call this function from any account.

  • The only requirement is knowing the target address (@owner), which is public on-chain.

Impact: High

  • Secrets expected to be private become publicly accessible.

  • This completely defeats the purpose of the SecretVault contract.

Proof of Concept

// Alice sets a secret in her vault
set_secret(&alice_signer, b"alice secret");
​
// Bob, who is not the owner, calls get_secret with Alice’s address
let secret = secret_vault::vault::get_secret(@alice);
​
// Bob successfully retrieves "alice secret"

Recommended Mitigation

Tie access control to the transaction signer, not a free address argument:

- public fun get_secret(caller: address): String acquires Vault {
- assert!(caller == @owner, NOT_OWNER);
- let vault = borrow_global<Vault>(@owner);
- vault.secret
- }
+ public fun get_secret(caller: &signer): String acquires Vault {
+ let addr = signer::address_of(caller);
+ let vault = borrow_global<Vault>(addr);
+ vault.secret
+ }

Finding 2: Secrets Overwritten on Reset

Description

The set_secret function creates a new Vault every time and stores it with move_to.
This overwrites any existing secret for the user. The design does not support multiple secrets or prevent accidental overwrites.

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: Medium

  • Every new call from the same user overwrites the previous secret.

Impact: Medium

  • Loss of previously stored secrets.

  • Users may believe multiple secrets are supported, leading to accidental data loss.

Recommended Mitigation

Implement update logic instead of overwriting blindly:

public entry fun set_secret(caller: &signer, secret: vector<u8>) acquires Vault {
let addr = signer::address_of(caller);
if (exists<Vault>(addr)) {
let vault = borrow_global_mut<Vault>(addr);
vault.secret = string::utf8(secret);
} else {
let vault = Vault { secret: string::utf8(secret) };
move_to(caller, vault);
}
event::emit(SetNewSecret {});
}

Finding 3: Hardcoded @owner

Description

The get_secret function references a hardcoded @owner address.
This is not defined in the contract and breaks the intended access control mechanism.
It also makes the module non-portable and confusing to audit.

let vault = borrow_global<Vault>(@owner);

Risk

Likelihood: High

  • The hardcoded reference is invalid if @owner is not properly defined.

  • Developers may incorrectly assume it refers to the caller.

Impact: Medium

  • Unexpected runtime errors.

  • Non-portable contract design.

Recommended Mitigation

Replace with dynamic address resolution using the caller signer:

- let vault = borrow_global<Vault>(@owner);
+ let addr = signer::address_of(caller);
+ let vault = borrow_global<Vault>(addr);

Finding 4: Weak Event Design

Description

The SetNewSecret event is emitted without including any metadata.
Events should carry information to make them meaningful for auditors and dApp frontends.

#[event]
struct SetNewSecret has drop, store {}

Risk

Likelihood: Medium

  • Events are always emitted but contain no useful information.

Impact: Low

  • Reduced auditability and dApp integration.

  • Makes it harder to track which account updated their secret.

Recommended Mitigation

Include useful fields in the event:

- #[event]
- struct SetNewSecret has drop, store {}
+ #[event]
+ struct SetNewSecret has drop, store {
+ owner: address,
+ timestamp: u64,
+ }

Conclusion

The current SecretVault contract has critical design issues that break confidentiality guarantees.
By fixing access control, preventing overwrites, removing hardcoded @owner, and improving events, the contract can achieve its intended purpose of secure secret storage.

Updates

Lead Judging Commences

bube Lead Judge 16 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Lack of signer check in `get_secret`

Support

FAQs

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