Unify wallet address validation and Ownable2Step for UUPS proxies (#329) #501

Merged
PlasticDigits merged 1 commit from issue-329-ownable2step-address-validate into main 2026-06-15 01:01:43 +00:00
PlasticDigits commented 2026-06-14 23:01:26 +00:00 (Migrated from gitlab.com)

Summary

Implements GitLab #329 (parent security review #325 recommendation #5; findings F-09 + partial F-01).

Indexer: Extract shared valid_0x_address20 into indexer/src/api_validate.rs and apply strict hex validation to arena wallet path/query handlers (arena_wallet_stats, podium donation donor, WarBow players). Non-hex 0x+42 strings return 400 invalid_address instead of 200 with empty stats.

Contracts: TimeArena and ReferralRegistry UUPS proxies now inherit Ownable2StepUpgradeable (__Ownable_init + __Ownable2Step_init). Ownership transfer requires acceptOwnership; UUPS upgradeTo remains onlyOwner. 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

Criterion Command / step Result
GET /v1/arena/wallet/0xGGGG…/stats → 400 invalid_address cd indexer && cargo test --test integration_stage2 (api_http_smoke) PASS
Valid addresses unchanged bash scripts/verify-wallet-profile-anvil.sh PASS
TimeArena + ReferralRegistry compile & tests with Ownable2Step cd contracts && forge test --match-contract "TimeArena|ReferralRegistry|DevStackIntegration" PASS
DeployDev + DevStackIntegration.t.sol bash scripts/verify-evm-dev-wallet-seed-anvil.sh PASS
Storage layout / upgrade notes documented security §329 PASS
Indexer clippy + unit tests cd indexer && cargo clippy --all-targets -- -D warnings && cargo test PASS
Integration stage2 cd indexer && cargo test --test integration_stage2 PASS
Full contract suite (non-fork) cd contracts && forge test --match-contract "TimeArena|ReferralRegistry|DevStackIntegration" (152 tests) PASS

Third-party verification

  1. Check out branch issue-329-ownable2step-address-validate.
  2. cd indexer && cargo clippy --all-targets -- -D warnings && cargo test && cargo test --test integration_stage2
  3. cd contracts && forge test --match-contract "TimeArena|ReferralRegistry|DevStackIntegration"
  4. bash scripts/verify-wallet-profile-anvil.sh and bash scripts/verify-evm-dev-wallet-seed-anvil.sh
  5. Optional attack smoke: curl -s -o /dev/null -w "%{http_code}" http://127.0.0.1:3100/v1/arena/wallet/0xgggggggggggggggggggggggggggggggggggggggg/stats → 400 with indexer running
  6. Forge: test_ownable2step_pending_owner_cannot_upgrade — pending owner cannot upgradeTo before acceptOwnership

Out of scope (per #325)

  • On-chain multisig / timelock wrappers for UUPS admin
  • Live mainnet proxy upgrade execution (documented path only)
## Summary Implements GitLab [#329](https://gitlab.com/PlasticDigits/yieldomega/-/issues/329) (parent security review [#325](https://gitlab.com/PlasticDigits/yieldomega/-/issues/325) recommendation **#5**; findings F-09 + partial F-01). **Indexer:** Extract shared `valid_0x_address20` into `indexer/src/api_validate.rs` and apply strict hex validation to arena wallet path/query handlers (`arena_wallet_stats`, podium donation `donor`, WarBow `players`). Non-hex `0x`+42 strings return **400** `invalid_address` instead of **200** with empty stats. **Contracts:** `TimeArena` and `ReferralRegistry` UUPS proxies now inherit **`Ownable2StepUpgradeable`** (`__Ownable_init` + `__Ownable2Step_init`). Ownership transfer requires `acceptOwnership`; UUPS `upgradeTo` remains `onlyOwner`. **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](docs/onchain/security-and-threat-model.md); invariants **`INV-INDEXER-329-ADDRESS-VALIDATE`**, **`INV-CONTRACTS-329-OWNABLE2STEP`**; play skill cross-link. ## Acceptance checklist | Criterion | Command / step | Result | |-----------|------------------|--------| | `GET /v1/arena/wallet/0xGGGG…/stats` → 400 `invalid_address` | `cd indexer && cargo test --test integration_stage2` (api_http_smoke) | PASS | | Valid addresses unchanged | `bash scripts/verify-wallet-profile-anvil.sh` | PASS | | `TimeArena` + `ReferralRegistry` compile & tests with Ownable2Step | `cd contracts && forge test --match-contract "TimeArena\|ReferralRegistry\|DevStackIntegration"` | PASS | | `DeployDev` + `DevStackIntegration.t.sol` | `bash scripts/verify-evm-dev-wallet-seed-anvil.sh` | PASS | | Storage layout / upgrade notes documented | [security §329](docs/onchain/security-and-threat-model.md#uups-ownership-upgrades-gitlab-329) | PASS | | Indexer clippy + unit tests | `cd indexer && cargo clippy --all-targets -- -D warnings && cargo test` | PASS | | Integration stage2 | `cd indexer && cargo test --test integration_stage2` | PASS | | Full contract suite (non-fork) | `cd contracts && forge test --match-contract "TimeArena\|ReferralRegistry\|DevStackIntegration"` (152 tests) | PASS | ## Third-party verification 1. Check out branch `issue-329-ownable2step-address-validate`. 2. `cd indexer && cargo clippy --all-targets -- -D warnings && cargo test && cargo test --test integration_stage2` 3. `cd contracts && forge test --match-contract "TimeArena|ReferralRegistry|DevStackIntegration"` 4. `bash scripts/verify-wallet-profile-anvil.sh` and `bash scripts/verify-evm-dev-wallet-seed-anvil.sh` 5. Optional attack smoke: `curl -s -o /dev/null -w "%{http_code}" http://127.0.0.1:3100/v1/arena/wallet/0xgggggggggggggggggggggggggggggggggggggggg/stats` → `400` with indexer running 6. Forge: `test_ownable2step_pending_owner_cannot_upgrade` — pending owner cannot `upgradeTo` before `acceptOwnership` ## Out of scope (per #325) - On-chain multisig / timelock wrappers for UUPS admin - Live mainnet proxy upgrade execution (documented path only)
PlasticDigits commented 2026-06-14 23:03:44 +00:00 (Migrated from gitlab.com)

Security review — MR !501

Commit reviewed: 9c89ffcc6d25f0ea4cb3071a688fe076a3e3da36
Scope: Ownable2Step migration on TimeArena / ReferralRegistry UUPS proxies; shared indexer valid_0x_address20 and strict hex validation on arena wallet stats, podium donation donor, and WarBow players query params; docs/invariants/tests.

Outcome: FINDINGS: 0 medium+

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):

  • Ownership transfer now requires acceptOwnership; pending owner cannot call upgradeTo (covered by new Forge tests).
  • _authorizeUpgrade remains onlyOwner; no access-control regression.
  • OpenZeppelin v5 ERC-7201 namespaced storage for _owner / _pendingOwner is layout-compatible with prior OwnableUpgradeable; fresh deploys call __Ownable_init + __Ownable2Step_init.

Indexer (api_validate.rs):

  • Stricter validation rejects malformed 0x+42 non-hex strings with 400 invalid_address instead of reaching DB binds.
  • All touched queries remain parameterized ($1 / ANY($1)); no SQL injection surface introduced or exposed.
  • WarBow players change (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.

## Security review — MR !501 **Commit reviewed:** `9c89ffcc6d25f0ea4cb3071a688fe076a3e3da36` **Scope:** Ownable2Step migration on `TimeArena` / `ReferralRegistry` UUPS proxies; shared indexer `valid_0x_address20` and strict hex validation on arena wallet stats, podium donation `donor`, and WarBow `players` query params; docs/invariants/tests. **Outcome:** `FINDINGS: 0` medium+ ### 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`):** - Ownership transfer now requires `acceptOwnership`; pending owner cannot call `upgradeTo` (covered by new Forge tests). - `_authorizeUpgrade` remains `onlyOwner`; no access-control regression. - OpenZeppelin v5 ERC-7201 namespaced storage for `_owner` / `_pendingOwner` is layout-compatible with prior `OwnableUpgradeable`; fresh deploys call `__Ownable_init` + `__Ownable2Step_init`. **Indexer (`api_validate.rs`):** - Stricter validation rejects malformed `0x`+42 non-hex strings with **400** `invalid_address` instead of reaching DB binds. - All touched queries remain parameterized (`$1` / `ANY($1)`); no SQL injection surface introduced or exposed. - WarBow `players` change (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.
PlasticDigits (Migrated from gitlab.com) merged commit 117d9aa327 into main 2026-06-15 01:01:43 +00:00
PlasticDigits commented 2026-06-15 01:01:44 +00:00 (Migrated from gitlab.com)

mentioned in commit 117d9aa327

mentioned in commit 117d9aa327f9b6da60afc817521f7b555701bd14
PlasticDigits commented 2026-06-15 01:22:39 +00:00 (Migrated from gitlab.com)

mentioned in issue #329

mentioned in issue #329
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
code/yieldomega!501
No description provided.