Secret Vault on Aptos

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

The set_secret function can be used by any user

Root + Impact

Description

The set_secret function is intended to can only be used by the Owner, but any user instead can use the set_secret function.

public entry fun set_secret(caller: &signer, secret: vector<u8>) {
// anyone can set the secret, as it is not capped to only owner can use it
let secret_vault = Vault { secret: string::utf8(secret) };
move_to(caller, secret_vault);
event::emit(SetNewSecret {});
}

Risk

Likelihood:

  • The user can always use the set_secret function

Impact:

  • As the Vault is only created for the signer, the owner's secret is not replaced by another user's secret, but the function of the protocol is disrupted as it only meant for the owner to use it.

Proof of Concept

Write the following test function on secret_vault.move :

#[test(owner = @0xcc, user = @0x1337)]
fun test_set_secret(owner: &signer, user: &signer) acquires Vault {
use aptos_framework::account;
// setting up the accounts
account::create_account_for_test(signer::address_of(owner));
account::create_account_for_test(signer::address_of(user));
// setting the owner secret
let secret: vector<u8> = b"setting a new secret";
set_secret(owner, secret);
let vault: &Vault = borrow_global<Vault>(signer::address_of(owner));
assert!(vault.secret == string::utf8(secret), 4);
// setting the user secret
let secret2: vector<u8> = b"i try to override it";
set_secret(user, secret2); // can be used by the user
let vault2: &Vault = borrow_global<Vault>(signer::address_of(user));
assert!(vault2.secret == string::utf8(secret2), 4); // checking the secret is as intended
assert!(vault1.secret != vault2.secret); // The secret is not the same, as it differs from address to address
}

Recommended Mitigation

Make sure that only owner can use the set_secret function, by using the assert!(bool, u64) function that it will revert if it's not the owner.

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 {});
}
Updates

Lead Judging Commences

bube Lead Judge 15 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.