[Bug]: Multicall3's protocol-registry membership lets any account write another user's LabelStore, permanently obstructing a name's transferability

[GHkrishna]

Component

Store

Priority

P2

What happened?

Every store write is gated on: "is the caller registered in the protocol registry?" isRegisteredAddress returns true for any address held under any key (DotnsProtocolRegistry.sol:90-91), and that is the whole gate on LabelStore.storeLabel (LabelStore.sol:178-181) and StoreFactory.deployLabelStoreFor (StoreFactory.sol:193-198).

Multicall3 is registered under the MULTICALL3 key (WireDeployments.s.sol:114, asserted at :183) — and Multicall3 makes any call anyone asks it to, with a plain CALL (Multicall3.sol:53). The target sees msg.sender == multicall3; the real caller is never checked.

So anyone can write into anyone's LabelStore, just by asking Multicall3 to do it.

And the write sticks. StoreUtils.writeLabel skips its write when an entry already exists (StoreUtils.sol:53), and isLocked means only "an entry exists" (LabelStore.sol:103-105). So a pre-written entry survives the protocol's own label write. storeLabel is single-write with no delete (LabelStore.sol:75), so it can never be renamed (not by the holder, not by the owner, not by governance).

How could one misue it:

Ask Multicall3 to call deployLabelStoreFor(victim) — creates the victim's store.
Ask Multicall3 to call storeLabel(bytes32(node), "") for a name the victim is about to get.
The victim registers the name. It works, they own it but the protocol's label write is skipped and the attacker's string stands.

Ps.: This could work with public addresses with a hint of what probable names they could register

What breaks

The name can never be transferred. The transfer hook reads the label from the sender's store, strips the TLD, and refuses an empty result (DotnsRegistrar.sol:453, :462). A string without the TLD makes every transfer revert InvalidLabel, and quoteTransferFee reverts too, so a client cannot even read the price.
labelOf returns the attacker's string (DotnsRegistrar.sol:158), so every UI and indexer shows the wrong name.

It also works on transfers, not just registrations

_syncRecipientStore writes into the recipient's store through the same helper (DotnsRegistrar.sol:297, :333-350). Poison Bob's store, and Alice → Bob still succeeds (pricing reads Alice's clean copy) but Bob → anyone reverts forever. This variant needs no mempool race, because recipient addresses like marketplace escrows, treasuries, known buyers can be known in advance. The registration variant does need one, but the reveal transaction is public and carries both label and owner.

Entries are keyed by (owner, node), so the attacker must know the future owner's address where this is targeted, not spray-and-pray. Against any publicly known address it is cheap, and the writes batch through Multicall3 itself. The only cost is a storage deposit per entry, locked forever since nothing can be deleted.

Expected behavior

A LabelStore entry should be writable only by the protocol component that legitimately owns that write, the registrar or an authorised controller and never by an arbitrary account.

1. Being present in the protocol registry should not confer write authority. The registry is an address book used for discovery; it should not double as an access-control list. A call forwarder in particular must never be a protocol principal.
2. A pre-existing entry at a node being registered is an anomaly, not something to honour. The registration path should either overwrite it or fail loudly, rather than silently accept a third party's string as the name's label.
3. No single unrecoverable write. A corrupted label entry should be repairable by the name's holder or by governance, rather than requiring a LabelStore implementation upgrade. (currentl the case)

Reproduction

Tests passing at:
test/unit/store/LabelStorePoisoning.t.sol

git checkout test/label-store-poisioning
forge clean && forge build
forge test --isolate --match-contract LabelStorePoisoningTests -vv

Additional context

Affected contracts.
contracts/store/LabelStore.sol, contracts/store/StoreFactory.sol, contracts/registry/DotnsProtocolRegistry.sol, contracts/utils/StoreUtils.sol, contracts/utils/Multicall3.sol, scripts/deploy/WireDeployments.s.sol. contracts/registrars/DotnsRegistrar.sol
is where the damage surfaces but is not itself at fault.

Why registration exists, and what it costs to remove. DEPLOYMENTS.md:69 gives the reason: the deployed Multicall3 is not the canonical 0xcA11… singleton, because it is deployed through the dotNS CREATE3 factory, so consumers cannot guess its address. The doc already offers the deployment manifest as an alternative source, and the manifest does publish it (deployments/paseo-assethub/420420417.json). So dropping the key is viable, but any SDK or frontend reading the address from the registry needs migrating first, and WireDeployments.s.sol:183, DEPLOYMENTS.md:264 and README.md:252 all reference the key.

Suggested fix

1. Stop using isRegisteredAddress for authorisation. Have LabelStore and StoreFactory check the specific keys they actually mean (REGISTRAR, CONTROLLER, POP_CONTROLLER) rather than "any address in the registry". Without it, the next contract registered for discovery inherits store-write authority the same way, and the registry's lack of a delete means that can never be revoked.
2. Unregister MULTICALL3. One line in WireDeployments.s.sol, plus the assertion, the checklist item, the README key list, and a consumer-migration note pointing at the manifest. (Can be done in combination with 1)
3. Revisit the silent skip in StoreUtils.writeLabel. For a fresh registration, an entry that already exists is an anomaly. The skip exists for: idempotency when a name transfers back to a prior holder, per the docstring. So the fix is to distinguish the two callers rather than to remove the skip: the registration path should overwrite or revert, and only the transfer path should tolerate an existing entry.

[re-gius]

Partially addressed by #278

[re-gius]

Current state after #261. Item 1 shipped earlier in #278: store writes and factory deploys are gated on the registrar and its controllers via StoreAuth.isStoreWriter rather than isRegisteredAddress. #261 covers item 2 for three of the four write paths: DotnsRegistrarController._completeRegistration, DotnsRegistrar._writeOwnerLabel and DotnsRegistry._writeSubnodeToStore now go through StoreUtils.writeNewLabel, which rejects an existing entry whose label differs from the one being written and tolerates a matching one so a previous holder can re-register; the transfer mirror keeps the tolerant writeLabel deliberately, so a transfer back to a prior holder does not fail on their own entry. From the suggested fixes, MULTICALL3 is no longer registered by the wiring stage, and DotnsProtocolRegistry.remove gives the registry a way to retire a key at all. Item 3 is untouched.

Left to do. A repair path for an already-corrupted entry: storeLabel is still single-write with no delete, so a wrong entry can only be cleared by a LabelStore implementation upgrade, which is exactly what item 3 says should not be required. The open design question is that letting holders rewrite their own label would let anyone set an arbitrary string as their name's label, so the likely shape is a protocol-gated resync from the canonical name, plus possibly a governance clear. The PoP issuance path is also still uncovered: DotnsPopController._writeRecord writes to LabelStore directly and returns early on a locked slot, and it is called from both _completeGatewayRegistration and _settlePendingLabel, so gateway mints and pending-claim settlement both still honour a pre-existing entry. Finally, MULTICALL3 remains registered on existing deployments, since #261 only stops fresh ones registering it; clearing it needs dotns-sdk and packages/ui reading the address from the manifest first, then a remove call.

[re-gius]

Downgrading to P2 since most of the items have been addressed.