Rendered from pearl/r/service_registry/FIXES.md at commit 6a510c665a53 in the project repository. The committed file is the source of truth; this page is a rendering of it.

service_registry — audit & remediation layer

Target: gno.land/r/g1ut6uspuh73e02yauxpmyt8g3wwddaq8utagvm3/service_registry (pearl-1) Upstream: SillyZir/service_registry @ 8f77853a77f595184e3fd35d53f222a13d898fa7 Audit date: 2026-09-21 Discovery record: DISCOVERY.md (committed alongside the source)

Layering

The port was applied in two strictly separated layers:

Mechanical port (port.py) — namespace rewrite plus the intermediate-gno-0.9 → pearl-1 API-era substitutions, every replacement pinned to an exact occurrence count. No behavioral change. This layer was run and its 21 upstream tests were green on the chain-matched toolchain before any remediation was written, so every later failure is unambiguously attributable to the remediation layer rather than to the API translation. port.py is now neutered: it exits immediately, because rerunning it would regenerate pre-remediation source over the fixed files.
Remediation (this document) — the audit-driven changes below. The committed .gno files are the deployable source of truth.

Port fidelity was re-verified independently of port.py on 2026-09-21: re-deriving the substitution counts directly from the upstream bytes at 8f77853 reproduces all eight pinned counts exactly (1, 1, 1, 12, 44, 10, 11, 2), so the transformation is total and deterministic with no unmatched residue.

The only API-era delta in the realm source is one import move and one symbol move — chain/runtime.PreviousRealm → chain/runtime/unsafe.PreviousRealm, identical stack-walker semantics. That symbol is then removed entirely by Y1 below, so the deployed realm no longer stack-walks for identity at all.

Findings

Severity per security.md § Severity calibration. RED = exploitable today or block-worthy operational concern; YELLOW = material unless the trust assumption is explicit and reasonable.

R1 — RED — namespace monopolization of a shared permissionless registry

MaxServices = 500 was the only bound on registration, and RegisterService is open to any address. A single funded key could claim 500 names for ~500 cheap transactions and permanently deny the registry to every other project. The retired reservation mechanism made this strictly worse rather than better: a deregistered name stays reserved for the squatter for ReservationPeriod (90 days), so the attacker does not even have to keep the entries alive to keep the names.

Fix: added MaxServicesPerOwner = 20, enforced in RegisterService and — at consent time — in AcceptOwnership, backed by an O(1) ownerServices map[address]int that is decremented on Deregister and on handoff, and pruned zero-free so the index cannot accumulate dead keys. Raised MaxServices 500 → 1000 so the global cap is a pure state bound rather than a de-facto anti-squat defense. 1000 is deliberately not higher: the linear scans over names (Deregister, ListServices, ListByType) must stay at a size one transaction can comfortably pay for.

Residual, stated honestly: this raises the cost of monopolization from one funded key to fifty, but does not eliminate sybil exhaustion. A permissionless shared namespace cannot fully solve that without a fee, a stake, or an allowlist — each of which would change this application's approved economic and trust model (and the ns* suite's paid/expiring model was evaluated and rejected for exactly that reason in DISCOVERY.md §3), and is therefore out of scope for this port.

Y1 — YELLOW (material) — latent Class-2 caller identity (stack-walking `caller()`)

func caller() address { return unsafe.PreviousRealm().Address() }        // non-crossing helper
func RegisterService(_ realm, name, pkgPath, ... string) { ... caller() ... }  // cur discarded

security.md lists unsafe.PreviousRealm() used as caller identity inside a non-crossing function as RED (Class 2 — it returns the realm before the most recent boundary, not the immediate caller). Every path this realm currently exposes reaches caller() directly from a crossing frame, so the resolved address was correct in practice — the finding is latent, not live. But all four entrypoints discarded their realm parameter entirely, so nothing structural tied identity to the caller; any future non-crossing exported helper calling caller() would have silently resolved its importer's caller instead of its importer.

The upstream code additionally called caller() three separate times inside a single RegisterService invocation (once for registrant, once for the reservation check, once for Owner), which is three independent opportunities for that drift.

Fix: all six entrypoints now take cur realm and derive identity once, inline, via cur.Previous().Address(). The caller() helper is deleted so it cannot be reintroduced.

This is the same finding and the same fix as in fee_split, timelock_guardian, upgrade_registry and permission_registry — the fifth occurrence of a single upstream idiom.

Y2 — YELLOW (material) — no stray-send guard

The realm holds no banker, exposes no payable path and has no withdrawal function. Coins attached to a MsgCall against any entrypoint would sit at the realm address permanently unrecoverable (security.md § operational treats fund-stranding as block-worthy).

Fix: rejectStraySend(cur) as the first statement of all six crossing entrypoints; the abort reverts the transfer. Guarded on cur.Previous().IsUserCall() — not IsUser() — because a MsgRun ephemeral can consume the OriginSend envelope before forwarding control. Fails open for realm-routed calls, whose attached send lands on the intermediary realm and never reaches here. Same helper as the four sibling realms.

Y3 — YELLOW (material) — unbounded `Render` output

Render walked every registered service with no cap — up to MaxServices rows, each carrying a name, type, path, address and description. Render is reachable by any viewer through gnoweb and vm/qrender, so unlike state growth its cost lands on third parties, not on whoever grew the state. Under R1's raised global cap this would have been 1000 rows.

Fix: output bounded by MaxRenderServices = 25, with an explicit truncation notice naming the bounded queries to use for complete data (ListServices / ListByType / GetService). The true total is printed in the header so truncation is never silent. Iteration is over the insertion-ordered names slice, never a map, so the shown subset is a deterministic function of state rather than of map layout.

ListServices and ListByType are deliberately left untruncated and carry a COST NOTE saying so: an integrator enumerating the registry needs the complete set, and a query caller pays for its own read. The asymmetry with Render is the point.

Y4 — YELLOW (material) — one-step `TransferOwnership` permanently bricks an entry

address.IsValid() only checks bech32 form. A well-formed but unowned destination passed the check and the transfer committed immediately, after which the entry could never again be updated, transferred or deregistered. Because it could never be deregistered it could never enter the reservation window either — so unlike a normal abandonment, the name became a permanent hole in a shared global namespace, with no expiry path at all. The original registrant's reclaim right, which the round-3 upstream audit had gone to some trouble to protect, was silently destroyed along with it.

Fix: two-step handoff — TransferOwnership nominates, AcceptOwnership (nominee-only) completes, CancelOwnershipTransfer (owner-only) withdraws; passing "" also clears. Nomination changes nothing: the sitting owner retains full control until consent. The nominee's quota is checked at consent time so a nomination can never push an account past MaxServicesPerOwner without that account agreeing. Self-nomination is rejected. A nomination cannot outlive its entry (Deregister clears it), so a stale nominee cannot seize a name that was deregistered and reclaimed. GetPendingOwner exposes the pending state.

Matches the AcceptTargetOwnership / AcceptAdmin pattern already shipped in timelock_guardian and permission_registry.

Y5 — YELLOW (material) — hand-rolled sanitizer permits markdown link/image phishing

func sanitize(s string) string {
	s = strings.ReplaceAll(s, "`", "'")
	s = strings.ReplaceAll(s, "|", "/")
	s = strings.ReplaceAll(s, "\n", " ")
	s = strings.ReplaceAll(s, "\r", " ")
	return s
}

This escaped backticks and pipes — table-breaking — but left [, ], (, ) and ! entirely live. Any address could register a service whose description was claim [here](https://evil.example/drain) or ![x](https://evil.example/pixel.png) and get a working, clickable markdown link or a remote-loading image rendered into the registry's own gnoweb page, under the registry's own apparent authority, in front of every viewer. For a discovery surface — a page whose entire purpose is to be read by people deciding what to integrate with — that is a direct phishing vector, not a cosmetic issue.

Fix: adopted the ecosystem sanitizer gno.land/p/nt/markdown/sanitize/v0 — sanitize.TableCell for table cells, sanitize.InlineText for the GetService summary — and deleted the hand-rolled sanitize(). TableCell escapes the full markdown metacharacter set (including [, ], (, ), !, . and |) and folds newlines.

Two ordering hazards were found and handled while adopting it:

Truncate before escaping, never after. TableCell escapes with backslashes; truncating the escaped string could sever a \x pair and leave a dangling backslash at the cell boundary. Render therefore applies truncate(desc, renderDescLen) to the RAW text and escapes the result. The reason is recorded in truncate's doc comment so it is not "tidied" back.
truncate was unguarded against small budgets. string(runes[:n-3]) panics on a negative index for n < 3. Added an explicit n < 4 guard. Unreachable at the current call site, fixed because the function is now load-bearing for a security control.

The sanitizer is deliberately applied once — it is not idempotent, and double-wrapping would render visible backslashes.

Y6 — YELLOW — `pkgPath` is an unverified claim, with no integrator contract

Resolve is the realm's whole reason to exist: other realms and tools call it to learn where a named service lives. But pkgPath is a caller-supplied string. This realm validates its structure (and the upstream round-3 audit did that well — traversal, doubled slashes, trailing slashes, hyphens and non-r/p roots are all refused) but it cannot and does not verify that the path exists, is deployed, or is controlled by the registrant. A consumer that treats a resolution as proof of provenance is trusting an assertion the registry never made.

DISCOVERY.md §2 records the contrast case: r/demo/defi/grc20reg is self-proving — it requires the registered token object to originate from the calling realm.

Deliberately NOT adopted. Requiring an equivalent proof here would mean only realms — never their human operators — could ever register a name. That is a fundamentally different application from the one approved, and changing it unilaterally would cross a hard scope boundary.

Fix (proportionate, in-scope): an explicit three-point INTEGRATOR CONTRACT doc block on Resolve, carried by reference on TryResolve:

A resolution is an attestation, not a proof — with the grc20reg contrast and the reason the stronger model was not adopted stated in the source itself.
A name is not an authorization — never grant a privilege, route a payment, or admit a caller because Resolve returned its path. Derive authority from your own crossing entrypoint's cur.Previous(), or from an explicit access-control realm. (This is the same consumer-side Class-2 hazard flagged as Y7 in permission_registry.)
The target can change — treat a resolution as valid only for the transaction that read it.

Same remedy shape as permission_registry Y7.

Y7 — YELLOW — silent repointing was unobservable

UpdateService may repoint a name at a different package path. That is a deliberate, necessary capability (services move; versions supersede), but the upstream realm emitted no events at all, so a bait-and-switch — build trust under an honest path, then repoint at a drainer — left no trace an integrator or indexer could watch for. The only way to detect it was to poll Resolve and diff.

Fix: chain.Emit on every state transition — ServiceRegistered, ServiceUpdated, OwnershipTransferProposed, OwnershipTransferred, OwnershipTransferCancelled, ServiceDeregistered. ServiceUpdated deliberately carries both oldpkgpath and pkgpath, so a repoint is a first-class, greppable fact in the transaction log rather than something a consumer has to reconstruct. Point 3 of the INTEGRATOR CONTRACT names these events as the intended detection mechanism.

Assessed and NOT changed

Expired retired tombstones are never swept — INFORMATIONAL, not material. Identical reasoning to permission_registry: retired grows monotonically with deregistration churn, but it is never iterated — only keyed lookups in RegisterService and a keyed write in Deregister — so there is no loop to blow up and no gas amplification onto other users. Every byte is paid for by the caller's own storage deposit, which is precisely the mechanism the chain prices state bloat with. A sweep index would add a new unbounded loop or a hard cap whose overflow behavior silently re-opens the squatting finding the reservation mechanism exists to close — strictly worse than the status quo. Added instead: an O(1) lazy reclaim in RegisterService that drops a lapsed tombstone the moment it is consulted. Recorded here so a future reviewer can disagree with the severity call on the evidence rather than rediscover it.
Reservation semantics — preserved exactly, and now regression-pinned. The upstream re-audit and round-3 fixes (finite ReservationPeriod; former owner and original registrant may reclaim in-window; an in-window re-register must preserve the original registrant so a hostile transferee cannot stamp themselves) are behaviorally untouched. Their tests were rewritten only to route ownership through the new two-step handoff. A new assertion pins the converse: a lapsed reservation confers nothing — the new registrant becomes the registrant, and the dead tombstone is reclaimed.
Identity forgery / privilege escalation — GREEN. services[name] is written only by RegisterService (owner = the resolved caller) and AcceptOwnership (nominee-gated); pendingOwners is written only behind an owner check. No function accepts an address as an identity parameter — TransferOwnership's newOwner is a nomination target, not an authenticated identity, and it cannot act until it authenticates itself in AcceptOwnership. No interface, callback or closure parameter anywhere, so Classes 1a/1b/3/4 do not apply.
Deployer / initialization — GREEN. init() only; no exported initializer, no re-initialization path, and the deployer receives no privilege whatsoever. There is no global admin, no pause, no upgrade hook and no owner-of-the-registry. The realm is genuinely permissionless, which is why R1 was the top finding.
Overflow — GREEN. ReservationPeriod as time.Duration is 7.776e15 ns, four orders of magnitude inside int64. Quota arithmetic is bounded by MaxServices. No other arithmetic.
Determinism — GREEN. All ordered output derives from the insertion-ordered names slice. No map is ranged anywhere in the realm; len(services) is the only map-wide read.
Injection — GREEN after Y5. Names and service types are [a-z0-9_]{1,64}; package paths are structurally validated per element; addresses are caller-derived or IsValid()-checked; free text goes through the ecosystem sanitizer; Render's path argument is never echoed.
names linear removal is O(MaxServices) = O(1000) string compares in a single Deregister transaction — bounded, paid by the deregisterer, and kept linear because ListServices documents registration order as part of its contract.
Resolve panics on unknown names — GREEN, by design. TryResolve is the non-panicking variant for consumers that must degrade gracefully; both are documented.

Test coverage

31 tests (from 21), all passing on the chain-matched pearl toolchain; gno lint clean; gno fmt clean; whole-pearl/ workspace regression green (12 packages).

New: TestRegister_PerOwnerQuota, TestQuota_IndexStaysZeroFree, TestRegister_GlobalCap, TestStraySendRejected, TestRender_Bounded, TestRender_RejectsMarkdownLinkInjection, TestTransferOwnership_TwoStep, TestTransferOwnership_Cancel, TestTransferOwnership_QuotaCheckedAtConsent, TestTransferOwnership_NominationDiesWithEntry.

Modified: TestTransferOwnership, TestReservation_OriginalRegistrantCanReclaim and TestReservation_RegistrantSurvivesHostileRecycle now route ownership through the complete two-step handoff; TestReservation_ExpiresAndFreesTheName additionally pins lapsed-tombstone reclaim; TestRender_ListsEntries pins that a short registry carries no truncation notice.

The pre-existing TestRender_SanitizesInjection was checked against the new sanitizer before the swap and is strictly stronger under it (TableCell escapes backticks and pipes with backslashes and folds newlines to spaces, so all three original assertions still hold) — the adoption strengthens rather than relaxes the suite.

Test-harness note pinned during this work: testing.SetRealm is frame-scoped. A caller identity set inside a helper function does not survive that helper's return. The fillFor helper carries this as a doc comment, and its callers re-arm the realm explicitly. Likewise testing.SkipHeights resets the entire test context, so TestReservation_ExpiresAndFreesTheName re-arms the caller realm after the time jump.