Unify wallet address validation and Ownable2Step for UUPS proxies (#329) #501
No reviewers
Labels
No labels
agent:fix_bugfix
agent:fix_conflicts
agent:fix_security
agent:gap_analysis
agent:implement
agent:open_issues
agents
agent:security_audit
agent:verify
area
timecurve
arena
arena-v2
audit
block:feature_review
block:security
bug
contracts
design
devops
docs
documentation
enhancement
epic
feature
frontend
indexer
low priority
megaeth
operations
ops
performance
QA
qa checklist
referrals
removal
security
smart-contracts
tech-debt
testing
time-arena
timecurve
visual-qa
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
code/yieldomega!501
Loading…
Reference in a new issue
No description provided.
Delete branch "issue-329-ownable2step-address-validate"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Implements GitLab #329 (parent security review #325 recommendation #5; findings F-09 + partial F-01).
Indexer: Extract shared
valid_0x_address20intoindexer/src/api_validate.rsand apply strict hex validation to arena wallet path/query handlers (arena_wallet_stats, podium donationdonor, WarBowplayers). Non-hex0x+42 strings return 400invalid_addressinstead of 200 with empty stats.Contracts:
TimeArenaandReferralRegistryUUPS proxies now inheritOwnable2StepUpgradeable(__Ownable_init+__Ownable2Step_init). Ownership transfer requiresacceptOwnership; UUPSupgradeToremainsonlyOwner. Multisig/timelock in-contract is explicitly out of scope (rejected on #325 until $100k monthly volume).Docs: Upgrade/storage notes in security-and-threat-model.md; invariants
INV-INDEXER-329-ADDRESS-VALIDATE,INV-CONTRACTS-329-OWNABLE2STEP; play skill cross-link.Acceptance checklist
GET /v1/arena/wallet/0xGGGG…/stats→ 400invalid_addresscd indexer && cargo test --test integration_stage2(api_http_smoke)bash scripts/verify-wallet-profile-anvil.shTimeArena+ReferralRegistrycompile & tests with Ownable2Stepcd contracts && forge test --match-contract "TimeArena|ReferralRegistry|DevStackIntegration"DeployDev+DevStackIntegration.t.solbash scripts/verify-evm-dev-wallet-seed-anvil.shcd indexer && cargo clippy --all-targets -- -D warnings && cargo testcd indexer && cargo test --test integration_stage2cd contracts && forge test --match-contract "TimeArena|ReferralRegistry|DevStackIntegration"(152 tests)Third-party verification
issue-329-ownable2step-address-validate.cd indexer && cargo clippy --all-targets -- -D warnings && cargo test && cargo test --test integration_stage2cd contracts && forge test --match-contract "TimeArena|ReferralRegistry|DevStackIntegration"bash scripts/verify-wallet-profile-anvil.shandbash scripts/verify-evm-dev-wallet-seed-anvil.shcurl -s -o /dev/null -w "%{http_code}" http://127.0.0.1:3100/v1/arena/wallet/0xgggggggggggggggggggggggggggggggggggggggg/stats→400with indexer runningtest_ownable2step_pending_owner_cannot_upgrade— pending owner cannotupgradeTobeforeacceptOwnershipOut of scope (per #325)
Security review — MR !501
Commit reviewed:
9c89ffcc6d25f0ea4cb3071a688fe076a3e3da36Scope: Ownable2Step migration on
TimeArena/ReferralRegistryUUPS proxies; shared indexervalid_0x_address20and strict hex validation on arena wallet stats, podium donationdonor, and WarBowplayersquery params; docs/invariants/tests.Outcome:
FINDINGS: 0medium+Summary
This MR is defensive hardening aligned with parent review #325 (F-09 / partial F-01). No new exploitable attack paths were identified in the diff.
Contracts (
Ownable2StepUpgradeable):acceptOwnership; pending owner cannot callupgradeTo(covered by new Forge tests)._authorizeUpgraderemainsonlyOwner; no access-control regression._owner/_pendingOwneris layout-compatible with priorOwnableUpgradeable; fresh deploys call__Ownable_init+__Ownable2Step_init.Indexer (
api_validate.rs):0x+42 non-hex strings with 400invalid_addressinstead of reaching DB binds.$1/ANY($1)); no SQL injection surface introduced or exposed.playerschange (reject on any invalid entry vs silent filter) tightens input handling; no cross-user impact.Prior MR security threads: none on this MR; nothing to re-report.
No inline finding threads — nothing at medium+ severity.
mentioned in commit
117d9aa327mentioned in issue #329