Root + Impact
Description
Only the designated owner (@owner address from Move.toml) should be able to store a secret in the Vault resource. Other accounts should have no ability to modify the owner’s stored secret.
Issue:
set_secret does not verify that the caller is the owner. This allows any account to call set_secret and overwrite the Vault resource belonging to the owner. In Move, move_to will replace the existing resource if one exists for that account, so this directly destroys the original secret.
public entry fun set_secret(caller:&signer, secret:vector<u8>) {
let secret_vault = Vault{secret: string::utf8(secret)};
// @> No ownership check here
move_to(caller, secret_vault);
event::emit(SetNewSecret {});
}
Risk
Likelihood:
-
Any user can call set_secret without restriction.
-
This will occur immediately when a malicious actor sees the module on-chain, since the entry function is public and permissionless.
Impact:
-
Permanent loss of the original secret stored by the owner.
-
Integrity breach: malicious data is stored in place of the true secret, breaking application trust.
Proof of Concept:
Test case:
#[test(owner = @0xcc, attacker = @0x999)]
fun test_overwrite_secret(owner: &signer, attacker: &signer) acquires Vault {
use aptos_framework::account;
// Create accounts for both the legitimate owner and an attacker
account::create_account_for_test(signer::address_of(owner));
account::create_account_for_test(signer::address_of(attacker));
// STEP 1: Owner stores their legitimate secret
let original_secret = b"legit secret";
set_secret(owner, original_secret);
// At this point, Vault is stored under @0xcc (the owner’s address)
// STEP 2: Attacker creates their *own* Vault
// PROBLEM: set_secret() uses the caller's signer address and does not check if they are the `owner`
let malicious_secret = b"hacked secret";
set_secret(attacker, malicious_secret);
// This creates a *separate* Vault resource at @0x999 (attacker’s address)
// STEP 3: Attacker retrieves their own Vault
let vault = borrow_global<Vault>(signer::address_of(attacker));
// STEP 4: This passes — attacker’s Vault contains their malicious secret
assert!(vault.secret == string::utf8(malicious_secret), 1000);
// 💡 Issue demonstrated:
// Any account can call set_secret() to store *their own* Vault.
// The spec requires that only the single `owner` address should be able to store a secret.
// This is a logical flaw — there’s no global ownership check in set_secret().
}
Test Results:
Auditing/FirstFlight/2025-07-secret-vault\$ aptos move test --filter test\_overwrite\_secret
INCLUDING DEPENDENCY AptosFramework
INCLUDING DEPENDENCY AptosStdlib
INCLUDING DEPENDENCY MoveStdlib
BUILDING aptos-secret-vault
Running Move unit tests
\[ PASS ] 0x234::vault::test\_overwrite\_secret
Test result: OK. Total tests: 1; passed: 1; failed: 0
{
"Result": "Success"
}
Recommended Mitigation:
Give permisson only to the owner and restrict to others...
public entry fun set_secret(caller:&signer, secret:vector<u8>) {
- Not checking any Owner this cause protocol (mentioned in the docs(.md))
let secret_vault = Vault{secret: string::utf8(secret)};
move_to(caller, secret_vault);
event::emit(SetNewSecret {});
}
After add the assert u can restrict the permission and give permission to Owner only
public entry fun set_secret(caller:&signer, secret:vector<u8>) {
+ assert!(signer::address_of(caller) == @owner, NOT_OWNER);
let secret_vault = Vault{secret: string::utf8(secret)};
move_to(caller, secret_vault);
event::emit(SetNewSecret {});
}
