Secret Vault on Aptos

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

Anyone can get the secret using public owner address breaking invariant

[H] Anyone can get the secret using public owner address breaking invariant

Description

The secret_vault::get_secret is build as follow

#[view]
public fun get_secret (caller: address):String acquires Vault{
assert! (caller == @owner,NOT_OWNER);
let vault = borrow_global<Vault >(@owner);
vault.secret
}

The function takes a public address as a parameter and then check if the address is the owner, if it is true then gives the secret, but the problem is that anyone can put the owner public address if they know it as it is public in the blockchain contrary to the private key.

Risk

It breaks the following invariant : Only the owner should be able to store a secret and then retrieve it later. Others should not be able to access the secret.
The secret is no more secret.

Proof of Concept

add the following test in secret_vault.move

#[test(user = @0x123, owner = @0xcc)]
fun test_get_secret(user: &signer, owner: &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 secret for the owner
let secret = b"i'm a secret";
set_secret(owner,secret);
// Get the owner address - EveryOne can have the public address of the owner in the blockchain !!
let owner_address = signer::address_of(owner);
// Call get secret - Anyone can just put the public address of the owner in get secret
let value = get_secret(owner_address);
assert!(value == string::utf8(secret), 4);
debug::print(&b"All tests passed!");
}

Recommended Mitigation

Instead of using an address as parameter use &signer it will directly put the caller of the function as the parameter and then check if it is the owner.
remove the #[view] and add the following lines to the function secret_vault::get_secret :

- #[view]
- public fun get_secret (caller: address):String acquires Vault{
+ public fun get_secret (caller: &signer):String acquires Vault{
- assert! (caller == @owner,NOT_OWNER);
+ assert! (signer::address_of(caller) == @owner, NOT_OWNER);
let vault = borrow_global<Vault >(@owner);
vault.secret
}
Updates

Lead Judging Commences

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

Lack of signer check in `get_secret`

Support

FAQs

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