Fix/stellar wave 790 791 792 793 - #934
Open
whisper011 wants to merge 7 commits into
Open
Conversation
…instead of only rejecting zero hash
…sthrough middleware
…ification and wire it into the router
|
@whisper011 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! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes four security/correctness gaps in
ip_registryandapi-server:ip_registry's admin was lazily set to the contract's own address (env.current_contract_address()) on firstcommit_ip/batch_commit_ip/batch_commit_ip_anonymouscall, which no external account can ever authorize as. This permanently bricked every admin-gated function (upgrade,set_notary_public_key,resolve_dispute,slash_stake,update_reputation,batch_update_reputation,nominate_arbitrator,open_arbitration). Added a realinitialize(admin)entrypoint (callable once, requires the admin's own auth) and aset_admin(new_admin)rotation path, removed the three self-referential init blocks, and fixedupgrade's vacuous self-comparison tautology.validate_upgradeonly rejected a zero WASM hash; any other hash passed regardless of whether it removed exported functions, storage keys, or renumbered error codes. It now takes a candidateUpgradeManifest(functions/storage-keys/error-codes) supplied by the off-chain tooling that built the candidate WASM, and rejects any upgrade that removes something the current contract relies on or reassigns an existing error code.request_queue_middlewarewas a literal passthrough; the fully-implementedRequestQueuewas never constructed or wired into the app. It's now constructed inmain.rsand applied as a real middleware layer, with queue-depth/wait-time metrics exposed.secret_keyparameter was never used), and the verification middleware was never mounted on the router. Replaced with real Ed25519 verification (mirroringauth::verify_stellar_signature), added a configurable replay-protection timestamp-skew window, and mounted the middleware on the IP/swap mutating endpoints.Test plan
No working Rust toolchain was available in this environment to run
cargo test/cargo build, so these changes are unverified by compilation or test execution — please run the suite in CI before merging.cargo test -p ip_registry— including new tests: non-admin rejection, real-admin success, pre-initialize rejection, double-initialize rejection, admin rotation,validate_upgraderejecting a removed function / renumbered error code and accepting a compatible manifestcargo test -p api-server— including newrequest_queueintegration tests (serves within capacity, rejects when full) andrequest_signingintegration tests (valid signature allowed, wrong-key/tampered-body/missing-header rejected)/ip/commit,/ip/transfer, and/swap/*write endpoints with a properly signed requestCloses #790
Closes #791
Closes #792
Closes #793