Secret Vault on Aptos

First Flight #46
Beginner FriendlyWallet
100 EXP
View results
Submission Details
Severity: low
Valid

Owner cannot update secret more than once

Root + Impact

Description

  • Once a user sets their secret using set_secret, they cannot update or overwrite it. Any further attempt results in a RESOURCE_ALREADY_EXISTS error.

public entry fun set_secret(caller:&signer,secret:vector<u8>){
let secret_vault = Vault{secret: string::utf8(secret)};
move_to(caller, secret_vault); // Fails if resource already exists
event::emit(SetNewSecret {});
}

Risk

Likelihood: Medium

  • This issue will occur at the time after the owner is needed to set the secret.

Impact: Medium

  • Owner is unable to update their secrets, which can impact usability.

Proof of Concept

  • Put the test_owner_can_only_set_once into the test section of secret_vault.move.

  • Run the test with aptos move test -f test_owner_can_only_set_once.

#[test(owner = @0xcc)]
#[expected_failure(major_status = 4004, location = Self)]
fun test_owner_can_only_set_once(owner: &signer) acquires Vault {
use std::string;
use aptos_framework::account;
account::create_account_for_test(signer::address_of(owner));
let secret = b"i'm a secret";
set_secret(owner, secret);
// Attempt to set a new secret, should fail
let secret_new = b"no more secrets";
set_secret(owner, secret_new);
}

Recommended Mitigation

There are multiple ways to fix this issue, here are two of them:

  1. Update the set_secret() to allow updating the secret if it already exists. This will avoid unnecessary removal and recreation.

- public entry fun set_secret(caller:&signer,secret:vector<u8>){
+ public entry fun set_secret(caller:&signer,secret:vector<u8>) acquires Vault{
+ if (exists<Vault>(signer::address_of(caller))) {
+ let secret_ref = &mut borrow_global_mut<Vault>(signer::address_of(caller)).secret;
+ *secret_ref = string::utf8(secret);
+ } else {
let secret_vault = Vault{secret: string::utf8(secret)};
move_to(caller,secret_vault);
+ };
event::emit(SetNewSecret {});
}
  1. Explicitly remove the resource if it already exists and recreate it.

- public entry fun set_secret(caller:&signer,secret:vector<u8>){
+ public entry fun set_secret_remove(caller: &signer, secret: vector<u8>) acquires Vault {
let secret_vault = Vault{secret: string::utf8(secret)};
+ if (exists<Vault>(signer::address_of(caller))) {
+ let _ = move_from<Vault>(signer::address_of(caller));
+ };
move_to(caller, secret_vault);
event::emit(SetNewSecret {});
}
Updates

Lead Judging Commences

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

The `secret` can not be updated

Support

FAQs

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