Secret Vault on Aptos

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

A non owner user can call set_secret()

set_secret() does not enforce that the caller is the owner + a non owner can call set_secret()

Description

  • set_secret() should enforce that the caller is the owner.

  • A non owner of the secret``vault module can call setsecret() to store a secret_vault resource under their account.

public entry fun set_secret(caller:&signer,secret:vector<u8>){
@> // no checks for whether caller is the owner here
let secret_vault = Vault{secret: string::utf8(secret)};
move_to(caller,secret_vault);
event::emit(SetNewSecret {});
}

Risk

Likelihood:

  • Occurs if a non owner user calls set_secret().

Impact:

  • A non owner of the secret``vault module can call setsecret() to store a secret_vault resource under their account.

Proof of Concept

#[test(owner = @0xcc, user = @0x123)]
fun test_anyone_can_store_secret_under_their_account(owner: &signer, user: &signer) acquires Vault{
use aptos_framework::account;
// Set up test environment
account::create_account_for_test(signer::address_of(owner));
account::create_account_for_test(signer::address_of(user));
// Create a new todo list for the user
let user_secret = b"user secret";
set_secret(user, user_secret);
// Verify the secret was added
let user_address = signer::address_of(user);
let user_vault = borrow_global<Vault>(user_address);
assert!(user_vault.secret == string::utf8(user_secret), 4);
}

Run with

aptos move test --filter test_anyone_can_store_secret_under_their_account

Output

INCLUDING DEPENDENCY AptosFramework
INCLUDING DEPENDENCY AptosStdlib
INCLUDING DEPENDENCY MoveStdlib
BUILDING aptos-secret-vault
Running Move unit tests
[ PASS ] 0x234::vault::test_anyone_can_store_secret_under_their_account
Test result: OK. Total tests: 1; passed: 1; failed: 0
{
"Result": "Success"
}

Recommended Mitigation

Check that caller is the owner in set_secret().

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 18 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.