Secret Vault

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

SetNewSecret Event is Uninformative and Lacks Indexed Data

Root + Impact

Description

  • The set_secret function emits a SetNewSecret event each time it is successfully called. Events are critical for off-chain services, indexers, and user interfaces to track contract activity without having to parse entire transactions.

  • The SetNewSecret event struct is defined as an empty struct. Consequently, it only signals that a secret was set, but provides no information about who set the secret. This makes the event nearly useless for any practical off-chain application.

// Root cause in the codebase with @> marks to highlight the relevant section
@>// events
#[event]
struct SetNewSecret has drop, store {
}@
// The emit call provides no useful data
public entry fun set_secret(caller:&signer,secret:vector<u8>){
let secret_vault = Vault{secret: string::utf8(secret)};
move_to(caller,secret_vault);
@>event::emit(SetNewSecret {});@
}

Risk

Likelihood:

  • Any off-chain indexer or dApp frontend listening to these events will be unable to attribute the action to a specific user based on the event data alone.

Impact:

  • The contract has poor observability, making it difficult to monitor user activity or build responsive applications on top of it.

  • Developers of services that integrate with this contract must resort to less efficient methods, like parsing full transaction data, to get information that should be present in the event.

Proof of Concept

This is a conceptual vulnerability related to design and observability. A proof of concept is explanatory:

An off-chain application or indexer would subscribe to SetNewSecret events from this module's handle. When a user 0x123 calls set_secret, the application would receive an event notification. However, the payload of the event would be empty. The application would know that a vault was created, but would have no way of knowing it was created by 0x123 from the event data itself. It would have to correlate the event with the transaction sender, defeating much of the purpose of emitting indexed event fields.

Recommended Mitigation

The event struct should be modified to include relevant data, at a minimum the address of the user who created the vault.

This is achieved by adding a user field of type address to the SetNewSecret struct. Then, when the event is emitted in the set_secret function, it should be populated with the caller's address. This makes the event useful for off-chain consumption.

- #[event]
- struct SetNewSecret has drop, store {
- }
+ #[event]
+ struct SetNewSecret has drop, store {
+ user: address,
+ }
- public entry fun set_secret(caller:&signer,secret:vector<u8>){
- let secret_vault = Vault{secret: string::utf8(secret)};
- move_to(caller,secret_vault);
- event::emit(SetNewSecret {});
- }
+ public entry fun set_secret(caller:&signer,secret:vector<u8>){
+ let user_address = signer::address_of(caller);
+ let secret_vault = Vault{secret: string::utf8(secret)};
+ move_to(caller,secret_vault);
+ event::emit(SetNewSecret { user: user_address });
+ }
Updates

Lead Judging Commences

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

Insufficient Data in `SetNewSecret` event

This is an Informational finding. It has no impact on the security of the protocol.

Support

FAQs

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