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

[H-1] Any user can create a secret

emerjux

Description

When calling the function set_secret, any user appart of the owner can create a secret for itself

Risk

Likelihood:

  • Any user with knowledge of the vault will be able to create a secret, not only the owner

Impact:

  • Most likely getting a free service for password storage

Proof of Concept

Add in the module secret_vault the following test

#[test(owner = @0xcc, user = @0xaa)]
#[expected_failure(abort_code = 1, location = secret_vault::vault)]
fun test_check_correct_string(owner: &signer, user: &signer) acquires Vault {
​
// Create the set_secret call with the owner
let secret_owner = b"i'm owner secret";
set_secret(owner, secret_owner);
​
// check the call with the owner
debug::print(&get_secret(@owner));
​
// This command should not succeed according to the documentation
// or at least not creating a secret as it is not the owner who does it
let secret_user = b"i'm user secret";
set_secret(user, secret_user);
​
// Check the values
let user_address = signer::address_of(user);
​
// this one should not be possible to print
let vault = borrow_global<Vault>(user_address);
debug::print(&vault.secret);
// This will fail as expected
debug::print(&get_secret(user_address));
​
}

Output

aptos move test
​
INCLUDING DEPENDENCY AptosFramework
INCLUDING DEPENDENCY AptosStdlib
INCLUDING DEPENDENCY MoveStdlib
BUILDING aptos-secret-vault
Running Move unit tests
[debug] "i'm owner secret"
[debug] "i'm user secret" <<<<< this line should not exist
[ PASS ] 0x234::vault::test_check_correct_string
Test result: OK. Total tests: 1; passed: 1; failed: 0
{
"Result": "Success"
}

Recommended Mitigation

Confirm that the caller of the set_secret is effectively the owner by following the same principle of the get_secret

public entry fun set_secret(caller:&signer,secret:vector<u8>){
+ let caller_address = signer::address_of(caller);
+ assert!(caller_address == @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 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.

Appeal created

emerjux Submitter
17 days ago
bube Lead Judge
16 days ago
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.