From 2e7cd34756c1ffe1f3459d582a8a637f6947623e Mon Sep 17 00:00:00 2001 From: LHerskind Date: Wed, 24 Jan 2024 09:41:10 +0000 Subject: [PATCH] chore: add comment around privacy-leaking singletons --- docs/docs/dev_docs/contracts/syntax/storage/main.md | 12 ++++++++++-- .../aztec/src/state_vars/immutable_singleton.nr | 6 +++++- .../aztec-nr/aztec/src/state_vars/singleton.nr | 6 +++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/docs/dev_docs/contracts/syntax/storage/main.md b/docs/docs/dev_docs/contracts/syntax/storage/main.md index b774ee0c682..263363832f1 100644 --- a/docs/docs/dev_docs/contracts/syntax/storage/main.md +++ b/docs/docs/dev_docs/contracts/syntax/storage/main.md @@ -288,7 +288,11 @@ As part of the initialization of the `Storage` struct, the `Singleton` is create As mentioned, the Singleton is initialized to create the first note and value. -When this function is called, a nullifier of the storage slot is created, preventing this Singleton from being initialized again. If an `owner` is specified, the nullifier will be hashed with the owner's secret key. It's crucial to provide an owner if the Singleton is associated with an account. Initializing it without an owner may inadvertently reveal important information about the owner's intention. +When this function is called, a nullifier of the storage slot is created, preventing this Singleton from being initialized again. + +:::danger Privacy-Leak +Beware that because this nullifier is created only from the storage slot without randomness it is "leaky". This means that if the storage slot depends on the an address then it is possible to link the nullifier to the address. For example, if the singleton is part of a `map` with an `AztecAddress` as the key then the nullifier will be linked to the address. +::: Unlike public states, which have a default initial value of `0` (or many zeros, in the case of a struct, array or map), a private state (of type `Singleton`, `ImmutableSingleton` or `Set`) does not have a default initial value. The `initialize` method (or `insert`, in the case of a `Set`) must be called. @@ -338,7 +342,11 @@ As part of the initialization of the `Storage` struct, the `Singleton` is create ### `initialize` -When this function is invoked, it creates a nullifier for the storage slot, ensuring that the ImmutableSingleton cannot be initialized again. If an owner is specified, the nullifier will be hashed with the owner's secret key. It is crucial to provide an owner if the ImmutableSingleton is linked to an account; initializing it without one may inadvertently disclose sensitive information about the owner's intent. +When this function is invoked, it creates a nullifier for the storage slot, ensuring that the ImmutableSingleton cannot be initialized again. + +:::danger Privacy-Leak +Beware that because this nullifier is created only from the storage slot without randomness it is "leaky". This means that if the storage slot depends on the an address then it is possible to link the nullifier to the address. For example, if the singleton is part of a `map` with an `AztecAddress` as the key then the nullifier will be linked to the address. +::: Set the value of an ImmutableSingleton by calling the `initialize` method: diff --git a/yarn-project/aztec-nr/aztec/src/state_vars/immutable_singleton.nr b/yarn-project/aztec-nr/aztec/src/state_vars/immutable_singleton.nr index 9178f2010ac..89135cbf1a2 100644 --- a/yarn-project/aztec-nr/aztec/src/state_vars/immutable_singleton.nr +++ b/yarn-project/aztec-nr/aztec/src/state_vars/immutable_singleton.nr @@ -32,7 +32,7 @@ impl ImmutableSingleton { note_interface: NoteInterface, ) -> Self { assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1."); - ImmutableSingleton { + Self { context: context.private, storage_slot, note_interface, @@ -40,6 +40,10 @@ impl ImmutableSingleton { } // docs:end:new + // The following computation is leaky - the storage slot can easily be "guessed" by an adversary + // by looking at the nullifier in the transaction data. + // This is especially dangerous for `maps` because the storage slot is often also identifies the + // actor that is executing the transaction. e.g, `map.at(msg.sender)` will leak `msg.sender`. pub fn compute_initialization_nullifier(self) -> Field { pedersen_hash([self.storage_slot], GENERATOR_INDEX__INITIALIZATION_NULLIFIER) } diff --git a/yarn-project/aztec-nr/aztec/src/state_vars/singleton.nr b/yarn-project/aztec-nr/aztec/src/state_vars/singleton.nr index 59472add997..7c005f24855 100644 --- a/yarn-project/aztec-nr/aztec/src/state_vars/singleton.nr +++ b/yarn-project/aztec-nr/aztec/src/state_vars/singleton.nr @@ -36,7 +36,7 @@ impl Singleton { note_interface: NoteInterface, ) -> Self { assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1."); - Singleton { + Self { context: context.private, storage_slot, note_interface, @@ -44,6 +44,10 @@ impl Singleton { } // docs:end:new + // The following computation is leaky - the storage slot can easily be "guessed" by an adversary + // by looking at the nullifier in the transaction data. + // This is especially dangerous for `maps` because the storage slot is often also identifies the + // actor that is executing the transaction. e.g, `map.at(msg.sender)` will leak `msg.sender`. pub fn compute_initialization_nullifier(self) -> Field { pedersen_hash([self.storage_slot], GENERATOR_INDEX__INITIALIZATION_NULLIFIER) }