Secret Vault

First Flight #46
Beginner FriendlyWallet
100 EXP
View results
Submission Details
Impact: low
Likelihood: medium
Invalid

get_secret aborts when no secret exists (no graceful “not set” path)

Root + Impact

Description

The get_secret function assumes that a Vault resource already exists for the given account. It directly calls borrow_global<Vault>(…), which aborts whenever the resource is not present in storage. This means that on any new account, or in cases where no secret has been set yet, attempts to read the secret will fail with an abort. Such behavior makes the contract less user-friendly and may confuse developers or integrators who expect a clear signal that no secret exists. A more robust design would handle the absence gracefully, either through a custom error code or by returning an optional value.

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

Root Cause

  • Missing existence check (exists<Vault>(addr)) before borrow_global.

Risk

Likelihood:

  • On any new account that hasn’t called set_secret, the first read will always abort.

  • Clients commonly probe for data before writing; those probes will routinely hit this path.

Impact:

  • Poor UX: first-time reads revert instead of returning a clear “not set” signal.

  • Harder client integration / testing due to unexpected aborts.

Proof of Concept

The following test method test_get_secret_aborts_if_not_exists demonstartes how getting an empty secret result on the call aborting.

#[test(owner = @0xcc)]
#[expected_failure] // We expect this test to abort
fun test_get_secret_aborts_if_not_exists(owner: &signer) acquires Vault {
use aptos_framework::account;
// Arrange: create the owner account, but DO NOT call set_secret
account::create_account_for_test(signer::address_of(owner));
// Act: try to read secret without ever publishing a Vault
// This will abort inside get_secret due to borrow_global<Vault> failing
let _ = get_secret(@owner);
}

Recommended Mitigation

  1. Either adding an explicit error (keep entry, authenticated):

+ const SECRET_NOT_SET: u64 = 2;
public entry fun get_secret(caller: &signer): String acquires Vault {
let addr = signer::address_of(caller);
- let vault = borrow_global<Vault>(addr);
+ assert!(exists<Vault>(addr), SECRET_NOT_SET);
+ let vault = borrow_global<Vault>(addr);
string::clone(&vault.secret)
}
  1. Or graceful view: return an Option<String> (no plaintext recommended in views)

+ use std::option::{Self, Option};
- #[view] public fun get_secret(addr: address): String acquires Vault {
- let v = borrow_global<Vault>(addr);
- v.secret
- }
+ #[view] public fun try_get_secret(addr: address): Option<String> acquires Vault {
+ if (!exists<Vault>(addr)) {
+ option::none<String>()
+ } else {
+ let v = borrow_global<Vault>(addr);
+ option::some(string::clone(&v.secret))
+ }
+ }
Updates

Lead Judging Commences

bube Lead Judge 6 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Lack of `Vault` existence check in `get_secret`

There is no security impact on the protocol, therefore this is an Informational finding. Also, it is a user mistake, if the user calls `get_secret` without first calling `set_secret`.

Support

FAQs

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