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

[H-1]: Critical Access Control Failure Allows Any User to Store a Vault

cosminmarian53

[H-1]: Critical Access Control Failure Allows Any User to Store a Vault

Description

The contract is intended to only allow the designated @owner to store a secret in a Vault resource at their own address.

However, the set_secret function is a public entry function with no access control. It allows any user to call it and create a Vault resource at their own address. This directly contradicts the fundamental security requirement of the contract. This single flaw is the root cause for the majority of the contract's problems.

// The get_secret function correctly checks for the owner...
#[view]
public fun get_secret(caller: address):String acquires Vault{
assert!(caller == @owner, NOT_OWNER);
let vault = borrow_global<Vault>(@owner);
vault.secret
}
​
// ...but the set_secret function has no such check, breaking the entire model.
public entry fun set_secret(caller:&signer, secret:vector<u8>){
@>// CRITICAL FLAW: No check to ensure caller is the owner.
let secret_vault = Vault{secret: string::utf8(secret)};
@>move_to(caller, secret_vault); // Any caller stores a vault at their own address.
event::emit(SetNewSecret {});
}

Risk

Likelihood: High

  • Any user can and will be able to call this public function. This isn't a complex edge case; it's a direct failure of the primary function.

Impact: High

This flaw has two major impacts:

  1. Loss of Data: Any non-owner who calls set_secret successfully stores a secret at their own address. However, because get_secret is hardcoded to read from the @owner address, the secret stored by the non-owner becomes permanently irretrievable. The data is effectively lost.

  2. Violation of Core Logic: The contract's main purpose is to be a private vault for the owner. By allowing anyone to create a vault, this purpose is completely defeated.

Proof of Concept

The vulnerability is confirmed by a logical walkthrough of the code's execution path:

  1. An attacker (any address that is not @owner) calls set_secret.

  2. The function executes without any access control checks.

  3. The line move_to(caller, secret_vault) stores a new Vault resource at the attacker's address.

  4. Later, no function can retrieve this secret. If the attacker calls get_secret, the assert!(caller == @owner, NOT_OWNER) check correctly causes an abort. If the owner calls get_secret, it will look for a vault at the @owner address, not the attacker's address, and will also fail (or retrieve a different vault if the owner had set one). The attacker's vault is orphaned.

Recommended Mitigation

The fix is to enforce ownership within set_secret and handle resource updates correctly. This single change fixes the access control, data loss, and update issues simultaneously.

- 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>) acquires Vault {
+ let owner_address = signer::address_of(caller);
+ // Enforce that only the owner can call this function.
+ assert!(owner_address == @owner, NOT_OWNER);
+
+ // Handle both creation and updates correctly.
+ if (exists<Vault>(owner_address)) {
+ let vault = borrow_global_mut<Vault>(owner_address);
+ vault.secret = string::utf8(secret);
+ } else {
+ let secret_vault = Vault{secret: string::utf8(secret)};
+ move_to(caller, secret_vault);
+ }
+ event::emit(SetNewSecret {});
+ }
Updates

Lead Judging Commences

bube Lead Judge 17 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Anyone can call `set_secret` function

In Move for Aptos, the term "owner" refers to a signer, which is a verified account that owns a given resource, has permission to add resources and the ability to grant access or modify digital assets. Following this logic in this contest, the owner is the account that owns `Vault`. This means that anyone has right to call `set_secret` and then to own the `Vault` and to retrieve the secret from the `Vault` in `get_secret` function. Therefore, this group is invalid, because the expected behavior is anyone to call the `set_secret` function.

Support

FAQs

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