Secret Vault on Aptos

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

The secret can be accessed by any user or attacker

Root + Impact

Description

The Vault struct and the secret is not private data, as the data is public on the blockchain. The get_secret function retrieve the secret from the storage, but the secret can be accessed by attacker when the attacker knows the owner address without the owner itself running the get_secret function.

struct Vault has key {
secret: String
}

Risk

Likelihood:

  • Vulnerability can always occur when the attacker knows the owner address.

Impact:

  • Disruption of the function of the smart contract itself, where it should be only the owner can access the secret, but instead anybody can access it.

Proof of Concept

Write the following test function on the secret_vault.move file.

#[test(owner = @0xcc, user = @0x123)]
fun test_secret_vault(owner: &signer, user: &signer) acquires Vault {
use aptos_framework::account;
// Set up test environment for accounts
account::create_account_for_test(signer::address_of(owner));
account::create_account_for_test(signer::address_of(user));
// Setting the secret for the Owner account
let secret = b"i'm a secret";
set_secret(owner, secret);
// Does not need to call the get_secret function
// Just needed the owner address, even works hard coded
let vault = borrow_global<Vault>(@0xcc);
// This returns true, as the secret is the same and not encrypted.
assert!(vault.secret == string::utf8(secret), 4);
}

or if the attacker want to use the get_secret function:

#[test(owner = @0xcc, user = @0x123)]
fun test_secret_vault(owner: &signer, user: &signer) acquires Vault {
use aptos_framework::account;
// Set up test environment for accounts
account::create_account_for_test(signer::address_of(owner));
account::create_account_for_test(signer::address_of(user));
// Setting the secret for the Owner account
let secret = b"i'm a secret";
set_secret(owner, secret);
let the_secret = get_secret(@0xcc);
// This returns true, as the secret is the same and not encrypted.
assert!(the_secret == string::utf8(secret), 4);
}

Recommended Mitigation

Consider to use encryption. All the data on the blockchain is public, and data that is sensitive should not be stored on the blockchain.

Updates

Lead Judging Commences

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