Skip to content

audit: security audit and storage optimization for stealth-registry - #35

Merged
truthixify merged 2 commits into
wraith-protocol:developfrom
thebabalola:audit/stealth-registry
Jun 2, 2026
Merged

audit: security audit and storage optimization for stealth-registry#35
truthixify merged 2 commits into
wraith-protocol:developfrom
thebabalola:audit/stealth-registry

Conversation

@thebabalola

Copy link
Copy Markdown
Contributor

Conducted a security audit of the stealth-registry contract. Improved scalability by switching to persistent storage and added a remove_keys function for privacy. Closes #2.

@drips-wave

drips-wave Bot commented May 29, 2026

Copy link
Copy Markdown

@thebabalola Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@thebabalola
thebabalola marked this pull request as ready for review May 29, 2026 21:07
@thebabalola

Copy link
Copy Markdown
Contributor Author

Hi @maintainer, I've completed the security audit and storage optimization for the stealth-registry contract. It's ready for review!

@truthixify
truthixify changed the base branch from main to develop June 1, 2026 15:10
@truthixify

Copy link
Copy Markdown
Contributor

The instance()persistent() migration is a real improvement and the remove_keys entrypoint is a sensible addition — both should land. But the audit deliverable here is lighter than what issue #2 asked for; could you expand it before we merge?

Specific gaps vs the issue spec:

  1. Location: Should be at stellar/stealth-registry/audits/2026-XX-author.md (in the audits/ subdirectory), matching the pattern PR Security audit of stealth-announcer Soroban contract #34 set for the announcer. Move AUDIT.md there.
  2. tests/audit.rs file: Each finding needs a reproducer test in a dedicated tests/audit.rs, like Security audit of stealth-announcer Soroban contract #34 did for the announcer. Right now the tests are inline in lib.rs and aren't labelled to which finding they prove.
  3. "Storage rent strategy" section: This is explicitly called out in the issue. With persistent storage your registrations no longer expire by archival, but Soroban still charges rent on TTL extension. A short section on whether (and how) we keep registrations alive would close the loop.
  4. Findings the issue asked you to look for that aren't covered yet:
    • Storage-key collision risk (can two (registrant, scheme_id) pairs collide via the DataKey packing?)
    • Replacement squatting (can an attacker pre-register a victim's slot? With require_auth() they can't, but it's worth documenting why.)
    • Scheme-ID forward compatibility (what happens with unknown scheme_ids?)
    • State exposure / privileged side channels
    • Replay protection across the write boundary (currently overwriting allowed — say so explicitly)

The code change (storage migration + remove_keys) is good. Just need the audit doc to match the rigor of #34.

Thanks @thebabalola — happy to re-review once you push the revisions.

@thebabalola

Copy link
Copy Markdown
Contributor Author

Hey @truthixify, thanks for the review! I've expanded the audit doc to cover everything requested:

  1. Moved the Report: Relocated to stellar/stealth-registry/audits/2026-06-thebabalola.md.
  2. Dedicated Reproducer Tests: Added tests/audit.rs with targeted, explicitly labeled tests for each finding (and moved standard tests to tests/registry.rs so they aren't mixed).
  3. Storage Rent: Documented that persistent TTL extension needs to be managed by clients/wallets.
  4. Collision Risk: Documented how XDR serialization prevents packing collisions.
  5. Replacement Squatting: Explicitly covered how require_auth() prevents this, with a reproducer test.
  6. Forward Compatibility: Tested and documented that unknown scheme_ids are accepted.
  7. Privileged Side Channels: Confirmed none exist.
  8. Replay Protection / Overwriting: Documented that intentional overwriting is allowed and protected by native sequence numbers.

Let me know if the rigor meets the bar for merging now!

@truthixify

Copy link
Copy Markdown
Contributor

The revision addresses everything I called out: audit doc moved to audits/2026-06-thebabalola.md, dedicated tests/audit.rs, reproducer tests for storage key collision / replacement squatting / scheme_id forward compat / replay protection. Verified locally that the merge cleanly preserves the wraith-names audit, gasless on-behalf flow, and the rest of recent develop. Merging. Thanks @thebabalola.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security audit of stealth-registry Soroban contract

2 participants