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

Hidden Performance and Security Issue from Implicit String Copying

r4y4n3

Root + Impact

Description

Normal Behavior

Read-only access functions should return references to data or explicitly indicate when copying occurs, allowing developers to understand performance implications and memory usage patterns.

Issue

The get_secret function implicitly copies the entire secret string on every call due to Move's automatic copying of Copy types from borrowed references. This creates unexpected performance overhead, multiple copies of sensitive data in memory, and violates the principle of least surprise for developers expecting reference-based access.

#[view]
public fun get_secret(caller: address): String acquires Vault {
assert!(caller == @owner, NOT_OWNER);
let vault = borrow_global<Vault>(@owner); // Returns &Vault
vault.secret // @> Implicitly copies String due to Copy trait - unexpected behavior!
}

Risk

Likelihood:

  • Every call to get_secret triggers implicit copying

  • Pattern occurs on every read operation

  • Developers are unaware of the copying behavior

  • Performance impact scales with secret size and call frequency

Impact:

  • Performance degradation: Unnecessary string copying on every read

  • Memory waste: Multiple copies of secrets in memory simultaneously

  • Security concern: Secrets persist longer in memory due to copies

  • Developer surprise: Unexpected copying behavior from apparent read-only operation

  • Scalability issues: Performance degrades with larger secrets or frequent access

Proof of Concept

The following tests demonstrate the implicit copying behavior and its implications:

#[test(owner = @0xcc)]
fun test_illegal_return_from_borrowed_resource(owner: &signer) acquires Vault {
account::create_account_for_test(signer::address_of(owner));
// Store a secret
let secret_data = b"test_secret_for_borrowing";
set_secret(owner, secret_data);
let owner_addr = signer::address_of(owner);
// VULNERABILITY: This call implicitly copies the entire string
let retrieved_secret = get_secret(owner_addr); // Creates a copy!
// Prove copying occurred (both values exist independently)
assert!(retrieved_secret == string::utf8(secret_data), 700);
// The original secret still exists in storage AND we have a copy
let vault = borrow_global<Vault>(owner_addr);
assert!(vault.secret == retrieved_secret, 701);
// Two independent string objects now exist in memory
}
​
#[test(owner = @0xcc)]
fun test_borrowing_rules_violation_detailed(owner: &signer) acquires Vault {
account::create_account_for_test(signer::address_of(owner));
set_secret(owner, b"test_data");
// Demonstrate what's actually happening:
let vault_ref = borrow_global<Vault>(@owner); // &Vault
// vault_ref.secret is &String, but returning as String triggers copy
// Move silently converts &String to String via Copy trait
// This is unexpected and potentially expensive
}
​
#[test(owner = @0xcc)]
fun test_correct_return_implementation(owner: &signer) acquires Vault {
account::create_account_for_test(signer::address_of(owner));
set_secret(owner, b"test_secret");
let owner_addr = signer::address_of(owner);
// Show the current behavior creates copies
let vault = borrow_global<Vault>(owner_addr);
let secret_copy = vault.secret; // Copy occurs here
// Better approaches:
// 1. Return reference: &String (if lifetime allows)
// 2. Explicit copying: string::clone(&vault.secret)
// 3. Verification pattern: don't return secrets at all
}

Recommended Mitigation

Verification Pattern (Best Security)

- #[view]
- public fun get_secret(caller: address): String acquires Vault {
+ public entry fun verify_secret(caller: &signer, claimed_secret: vector<u8>): bool acquires Vault {
+ assert!(signer::address_of(caller) == @owner, NOT_OWNER);
let vault = borrow_global<Vault>(@owner);
- vault.secret
+ vault.secret == string::utf8(claimed_secret) // No copying, just comparison
}
Updates

Lead Judging Commences

bube Lead Judge 11 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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