diff --git a/tests/samourai-crew/AUDIT_SECURITY_2026-06-02.md b/tests/samourai-crew/AUDIT_SECURITY_2026-06-02.md new file mode 100644 index 0000000..a3a16e7 --- /dev/null +++ b/tests/samourai-crew/AUDIT_SECURITY_2026-06-02.md @@ -0,0 +1,579 @@ +# Security Audit Report — `examples/gno.land` + +**Date:** 2026-06-02 +**Scope:** packages `p/` and realms `r/` outside quarantine +**Result:** 7 HIGH · 11 MEDIUM + +--- + +## Table of contents + +- [HIGH Findings](#high-findings) + - [H1 — Blog: moderator/commenter revocation permanently broken](#h1--blog-moderatorcommenter-revocation-permanently-broken) + - [H2 — GovDAO: bootstrap window allows anyone to replace the DAO implementation](#h2--govdao-bootstrap-window-allows-anyone-to-replace-the-dao-implementation) + - [H3 — GovDAO: voting power computed live (no snapshot)](#h3--govdao-voting-power-computed-live-no-snapshot) + - [H4 — Atomicswap GRC20: shared balance across swaps → permanent fund lock](#h4--atomicswap-grc20-shared-balance-across-swaps--permanent-fund-lock) + - [H5 — GovDAO: invitation points checked at creation, not at execution](#h5--govdao-invitation-points-checked-at-creation-not-at-execution) + - [H6 — GhVerify: handle/address mapping freely overwritable](#h6--ghverify-handleaddress-mapping-freely-overwritable) + - [H7 — Profile: missing `IsUserCall()` guard → any realm can write a profile](#h7--profile-missing-isusercall-guard--any-realm-can-write-a-profile) +- [MEDIUM Findings](#medium-findings) + - [M1 — Treasury: inherits bootstrap bypass (linked to H2)](#m1--treasury-inherits-bootstrap-bypass-linked-to-h2) + - [M2 — CommonDAO: vote rewrite without finality + `ErrVoteExists` dead code](#m2--commondao-vote-rewrite-without-finality--errvoteexists-dead-code) + - [M3 — `once.DoErr`: returns `nil` if `fn()` panics](#m3--oncedoerr-returns-nil-if-fn-panics) + - [M4 — Sys/Users: `RegisterUserIgnoreCanonical` accessible to all whitelisted controllers](#m4--sysusers-registeruserignorecanonical-accessible-to-all-whitelisted-controllers) + - [M5 — Blog: admin transfer without governance](#m5--blog-admin-transfer-without-governance) + - [M6 — GovDAO: T3→T1 promotion costs no invitation points](#m6--govdao-t3t1-promotion-costs-no-invitation-points) + - [M7 — Disperse: silent parsing error → zero-amount transfers](#m7--disperse-silent-parsing-error--zero-amount-transfers) + - [M8 — Profile: arbitrary field names accepted](#m8--profile-arbitrary-field-names-accepted) + - [M9 — GRC20Factory Faucet: unlimited mint](#m9--grc20factory-faucet-unlimited-mint) + - [M10 — Events: `/admin` route accessible without authentication](#m10--events-admin-route-accessible-without-authentication) + - [M11 — Boards2/Blog: content injection via unfiltered markdown](#m11--boards2blog-content-injection-via-unfiltered-markdown) +- [Executive Summary](#executive-summary) + +--- + +## HIGH Findings + +--- + +### H1 — Blog: moderator/commenter revocation permanently broken + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/gnoland/blog/admin.gno` | +| **Lines** | 38–45 (removal functions), 119–127 (check functions) | +| **Severity** | HIGH | +| **Category** | Privilege Escalation / Access Control Bypass | +| **Confidence** | 0.99 | + +**Description:** + +`AdminRemoveModerator` and `ModDelCommenter` call `BPTree.Set(addr.String(), false)` to revoke access. But `isModerator` and `isCommenter` only test the boolean `found` returned by `BPTree.Get()`: + +```go +func isModerator(addr address) bool { + _, found := moderatorList.Get(addr.String()) + return found // true even if the stored value is false! +} +``` + +`BPTree.Set(key, false)` updates the value but **does not delete the key**. `Get()` therefore returns `(false, true)` — `found` stays `true`. Revocation has no effect. The code even contains a comment `// FIXME: delete instead?` acknowledging the problem. + +**Exploit:** + +1. Admin calls `AdminAddModerator(moderatorAddr)`. +2. Moderator misbehaves; admin calls `AdminRemoveModerator(moderatorAddr)`. +3. Despite the "removal", `isModerator(moderatorAddr)` still returns `true`. +4. The ex-moderator continues calling `ModAddPost`, `ModEditPost`, `ModRemovePost`, etc. indefinitely. +5. There is no way to revoke access without redeploying the realm. + +The same bug identically affects `ModDelCommenter` / `isCommenter`. + +**Fix:** + +```go +func AdminRemoveModerator(_ realm, addr address) { + assertIsAdmin() + moderatorList.Remove(addr.String()) +} + +func ModDelCommenter(_ realm, addr address) { + assertIsModerator() + commenterList.Remove(addr.String()) +} +``` + +--- + +### H2 — GovDAO: bootstrap window allows anyone to replace the DAO implementation + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/gov/dao/proxy.gno` | +| **Lines** | 168–178 | +| **Severity** | HIGH | +| **Category** | Access Control / Governance Takeover | +| **Confidence** | 0.95 | + +**Description:** + +`UpdateImpl` replaces the DAO implementation and rewrites the `allowedDAOs` whitelist. It relies on `InAllowedDAOs` which returns `true` when `allowedDAOs` is empty (intentional initialization logic). This window is never programmatically closed. If the bootstrap MsgRun is delayed or fails, `allowedDAOs` stays empty and `UpdateImpl` is callable by any realm. + +**Exploit:** + +1. Chain is deployed with `allowedDAOs = nil`. +2. The bootstrap MsgRun (meant to populate the list) is delayed or fails. +3. Attacker deploys `r/evil/dao` implementing the `DAO` interface. +4. Attacker calls `dao.UpdateImpl(cross(cur), NewUpdateRequest(evilDAO, []string{"gno.land/r/evil/dao"}))` — `InAllowedDAOs("")` returns `true` (empty list). +5. The DAO implementation is now under attacker control. All existing proposals can be force-executed; all future proposals can be approved or rejected at will. + +**Fix:** + +Add an `initialized bool` flag set to `true` on the first non-nil call to `AllowedDAOs`. After initialization, `InAllowedDAOs` must unconditionally return `false` for any caller absent from the list. + +--- + +### H3 — GovDAO: voting power computed live (no snapshot) + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/gov/dao/v3/impl/types.gno` | +| **Lines** | 88–118 (`totalPower`, `votePowerPercent`) | +| **Severity** | HIGH | +| **Category** | Governance Manipulation | +| **Confidence** | 0.88 | + +**Description:** + +`YesPercent` and `NoPercent` are computed dynamically from the **current** member roster at `PreExecuteProposal` time, not at the time votes were cast. If T1 members are removed between vote and execution (via another proposal), the denominator shrinks, artificially inflating the YES percentage of remaining votes. + +**Exploit:** + +1. Proposal `P1` to remove 40% of T1 members receives ~60% YES. +2. A legitimate proposal `P2` removes some T1 NO-voters and executes first. +3. `PreExecuteProposal` is called for `P1`: `totalPower` is recomputed on the reduced roster. The 60% YES now represents >66.66% of the reduced total. +4. `P1` passes — even though it did not have a supermajority at vote time. + +**Fix:** + +Snapshot the total voting power at proposal creation (`PostCreateProposal`) and use that snapshot in `votePowerPercent`. This is the standard approach (e.g. Compound's GovernorBravo). + +> Note: the code itself documents this with a comment linking to the open discussion at +> https://github.com/gnolang/gno/pull/5271#discussion_r2952523023 + +--- + +### H4 — Atomicswap GRC20: shared balance across swaps → permanent fund lock + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/demo/defi/atomicswap/atomicswap.gno` | +| **Lines** | 109–133 (`NewCustomGRC20Swap`) | +| **Severity** | HIGH | +| **Category** | Fund Theft / Accounting Error | +| **Confidence** | 0.85 | + +**Description:** + +`NewCustomGRC20Swap` creates a `sendFn` closure capturing the `allowance` amount at swap creation. All swaps for the same GRC20 token share the **same global realm balance**. If multiple swaps coexist and one exhausts the balance (via Claim or Refund), the others can no longer be fulfilled — their `sendFn` panics, the transaction reverts, but the swap remains in a permanently unclaimable state. + +**Exploit (fund lock):** + +1. Alice creates a swap for 100,000 TST. +2. Bob creates a swap for 100,000 TST. Realm balance: 200,000 TST. +3. Bob claims his swap. Balance: 100,000 TST. +4. Alice tries to refund her swap: `sendFn` tries to transfer 100,000 TST — OK if balance sufficient. +5. Critical scenario: if a third party creates and claims a swap just before Alice, the balance drops to 0. Alice's `sendFn` panics indefinitely → her funds are permanently locked in the realm. + +**Fix:** + +Use a sub-teller per swap (`token.RealmSubTeller(swapID)`) to isolate each GRC20 escrow. Each `sendFn` only draws from its dedicated sub-account. + +--- + +### H5 — GovDAO: invitation points checked at creation, not at execution + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/gov/dao/v3/impl/prop_requests.gno` | +| **Lines** | 76–90 (`NewAddMemberRequest`) | +| **Severity** | HIGH | +| **Category** | Governance Manipulation | +| **Confidence** | 0.85 | + +**Description:** + +`NewAddMemberRequest` checks `member.InvitationPoints <= 0` at proposal **creation**, but the deduction `RemoveInvitationPoint()` only happens at **execution**. A member with 1 point can submit multiple proposals simultaneously before any executes — each creation passes the check (1 > 0), but the first execution will deduct the point and subsequent ones will panic on revert. Result: 1 invitation point = potentially N members added if timing is favorable. + +**Exploit:** + +1. Alice has exactly 1 invitation point. +2. Alice submits `NewAddMemberRequest(addr1)` and `NewAddMemberRequest(addr2)` before either executes. +3. Both creations pass the check (1 > 0). +4. `P1` executes: deducts 1 point → `addr1` added. +5. `P2` executes: panics (0 points) → reverts, `addr2` not added. +6. Alice used 1 point but could potentially get two members added if execution order is manipulated. + +**Fix:** + +Move the check and deduction into the execution callback, re-reading `GetMember` on the live pointer: + +```go +cb := func(cur realm) error { + m, _ := memberstore.Get(0, cur).GetMember(proposerAddr) + if m == nil || m.InvitationPoints <= 0 { + return errors.New("proposer no longer has invitation points") + } + m.RemoveInvitationPoint() + return memberstore.Get(0, cur).SetMember(tier, addr, memberByTier(tier)) +} +``` + +--- + +### H6 — GhVerify: handle/address mapping freely overwritable + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/gnoland/ghverify/contract.gno` | +| **Lines** | 66–67 (`postHandler.Handle`) | +| **Severity** | HIGH | +| **Category** | Verification Bypass / Identity Hijacking | +| **Confidence** | 0.85 | + +**Description:** + +`handleToAddressMap.Set()` and `addressToHandleMap.Set()` silently overwrite any existing entry without conflict-checking. No guard prevents: +- An already-mapped handle from being remapped to a different address. +- An already-verified address from being re-verified with a different handle. + +An attacker with temporary access to Alice's GitHub account can submit a verification request from their own Gno address, silently moving the mapping `alice → g1attacker` and leaving `g1alice → alice` in an orphaned state. + +**Exploit:** + +1. Alice (`g1alice`) is verified as GitHub `alice`. Mappings: `alice → g1alice`, `g1alice → alice`. +2. Attacker (`g1bob`) temporarily takes control of GitHub `alice`. +3. Attacker calls `RequestVerification("alice")`. +4. Oracle confirms: `g1bob` is linked to `alice`. +5. Callback writes: `alice → g1bob`. The old `g1alice → alice` entry persists (inconsistent state). +6. `GetAddressByHandle("alice")` now returns `g1bob`. + +**Fix:** + +```go +// Clean up old handle→address entry if this address was already verified +if oldHandle, ok := addressToHandleMap.Get(task.gnoAddress); ok { + handleToAddressMap.Remove(oldHandle.(string)) +} +// Clean up old address→handle entry if this handle was already mapped +if oldAddr, ok := handleToAddressMap.Get(task.githubHandle); ok { + addressToHandleMap.Remove(oldAddr.(string)) +} +handleToAddressMap.Set(task.githubHandle, task.gnoAddress) +addressToHandleMap.Set(task.gnoAddress, task.githubHandle) +``` + +--- + +### H7 — Profile: missing `IsUserCall()` guard → any realm can write a profile + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/demo/profile/profile.gno` | +| **Lines** | 72–115 (`SetStringField`, `SetIntField`, `SetBoolField`) | +| **Severity** | HIGH | +| **Category** | Profile Spoofing / Caller Identity Confusion | +| **Confidence** | 0.82 | + +**Description:** + +Profile setters use `cur.Previous().Address()` as the identity key without checking `cur.Previous().IsUserCall()`. When an intermediary realm calls the setter, `cur.Previous()` is the realm's address — not the signing EOA. Any realm can therefore write a profile field under **its own realm address**, which can mislead UIs displaying profiles associated with known realm addresses. + +**Fix:** + +Add at the start of each setter: + +```go +if !cur.Previous().IsUserCall() { + panic("profile: only direct EOA calls allowed") +} +``` + +--- + +## MEDIUM Findings + +--- + +### M1 — Treasury: inherits bootstrap bypass (linked to H2) + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/gov/dao/v3/treasury/treasury.gno` | +| **Lines** | 60–86 (`SetTokenKeys`, `Send`) | +| **Severity** | MEDIUM | +| **Category** | Access Control | +| **Confidence** | 0.90 | + +**Description:** + +`treasury.Send` and `treasury.SetTokenKeys` authenticate via `dao.InAllowedDAOs()` — the same function with the empty-list bug described in H2. During the bootstrap window (or if `allowedDAOs` is reset to empty), any realm can call `treasury.Send` and drain the treasury without governance approval. + +**Fix:** Linked to the H2 fix. Additionally, add an explicit `cur.IsCurrent()` assertion at the start of `Send` and `SetTokenKeys`. + +--- + +### M2 — CommonDAO: vote rewrite without finality + `ErrVoteExists` dead code + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/p/nt/commondao/v0/record.gno` | +| **Lines** | 100–113 (`AddVote`) | +| **Severity** | MEDIUM | +| **Category** | Governance Manipulation | +| **Confidence** | 0.85 | + +**Description:** + +`AddVote` explicitly allows vote rewriting: if a member votes again, their old vote is replaced and the old choice counter is decremented. `ErrVoteExists` is defined in `record.gno` but **never used** in the call path — dead code signaling that the original behavior (rejecting re-votes) was abandoned without a clear replacement policy. + +**Exploit:** + +A member waits to observe the public tally (all on-chain state is readable), then switches their vote just before the deadline to flip a close result. + +**Fix:** + +Explicitly choose and document one of two options: +- **Option A (lock votes):** Return `ErrVoteExists` from `Vote()` if `p.record.HasVoted(member)` is true. +- **Option B (allow changes):** Delete `ErrVoteExists` and document that vote-changing is intentional. + +--- + +### M3 — `once.DoErr`: returns `nil` if `fn()` panics + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/p/moul/once/once.gno` | +| **Lines** | 32–39 (`DoErr`) | +| **Severity** | MEDIUM | +| **Category** | Invariant Break | +| **Confidence** | 0.85 | + +**Description:** + +`DoErr` uses `defer func() { o.done = true }()` before calling `fn()`. If `fn()` panics, the defer runs anyway: `o.done = true` and `o.err = nil` (zero value, never assigned). A caller wrapping `Authorize` in a `recover` observes a `nil` return — as if the action succeeded — when it actually panicked mid-way. The one-shot action is permanently marked as executed and can never be retried. + +**Fix:** + +```go +func (o *Once) DoErr(fn func() error) (err error) { + if o.done { + return o.err + } + panicked := true + defer func() { + if panicked { + o.err = errors.New("once: fn panicked") + } + o.done = true + }() + o.err = fn() + panicked = false + return o.err +} +``` + +--- + +### M4 — Sys/Users: `RegisterUserIgnoreCanonical` accessible to all whitelisted controllers + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/sys/users/store.gno` | +| **Lines** | 137–159 | +| **Severity** | MEDIUM | +| **Category** | Access Control | +| **Confidence** | 0.83 | + +**Description:** + +`RegisterUserIgnoreCanonical` disables canonical name collision detection. It is protected by the same `controllers` whitelist as `RegisterUser`. If a future malicious controller is added via governance, it can call the bypass variant and register confusable names (e.g. `vital1k` vs `vitalik`) without any detection. + +**Fix:** + +Create a separate higher-privilege whitelist for `RegisterUserIgnoreCanonical`, or restrict the call to direct governance only (no delegated controller). + +--- + +### M5 — Blog: admin transfer without governance + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/gnoland/blog/admin.gno` | +| **Lines** | 26–29 | +| **Severity** | MEDIUM | +| **Category** | Ownership Hijacking | +| **Confidence** | 0.82 | + +**Description:** + +`AdminSetAdminAddr` transfers blog admin rights in a **single transaction**, without timelock, without a GovDAO proposal, without confirmation. The initial `adminAddr` is the T1 GovDAO multisig. A compromised admin account (or a leaked multisig key) can instantly and irrevocably transfer full blog control to an arbitrary address. + +**Fix:** + +Route the admin change through a GovDAO proposal, following the `NewPostProposalRequest` pattern already used for posts. + +--- + +### M6 — GovDAO: T3→T1 promotion costs no invitation points + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/gov/dao/v3/impl/prop_requests.gno` | +| **Lines** | 119–143 (`NewPromoteMemberRequest`) | +| **Severity** | MEDIUM | +| **Category** | Governance Manipulation | +| **Confidence** | 0.82 | + +**Description:** + +Adding a T1 member directly via `NewAddMemberRequest` costs 1 invitation point. But promoting them from T3 to T1 via `NewPromoteMemberRequest` costs **nothing**. Workaround: add someone at T3 (cost: 1 point via `AddMember`), then promote to T1 via proposal with no additional cost — bypassing the invitation economy for higher tiers. + +**Fix:** + +Either deduct invitation points proportional to the target tier on promotion, or explicitly document and enforce that the promotion path is intentionally exempt from invitation points. + +--- + +### M7 — Disperse: silent parsing error → zero-amount transfers + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/demo/disperse/util.gno` | +| **Lines** | 36–51 (`parseTokens`) | +| **Severity** | MEDIUM | +| **Category** | Accounting Error / Input Validation | +| **Confidence** | 0.82 | + +**Description:** + +`strconv.Atoi(amountStr)` returns `(0, error)` when `amountStr` is empty (case of a malformed token like `"FOO"`). The error is discarded with `_`. The amount `0` passes the `amount < 0` check, and `TransferFrom` with amount 0 succeeds silently. An upstream protocol relying on transaction success (non-panic) as proof of payment can be deceived. + +**Fix:** + +```go +amount, err := strconv.Atoi(amountStr) +if err != nil || amount <= 0 { + panic("disperse: invalid token amount") +} +``` + +--- + +### M8 — Profile: arbitrary field names accepted + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/demo/profile/profile.gno` | +| **Lines** | 72–85 (`SetStringField`) | +| **Severity** | MEDIUM | +| **Category** | Data Integrity | +| **Confidence** | 0.80 | + +**Description:** + +No field name validation on write. The `stringFields`, `intFields`, `boolFields` maps define valid fields on the getter side, but the setter accepts any string. Anyone can write persistent arbitrary entries under names like `"Admin"`, `"DisplayName:injected"`, creating misleading data in the store. + +**Fix:** + +Validate `field` against the allowlist before any write: + +```go +if _, ok := stringFields[field]; !ok { + panic("profile: unknown field: " + field) +} +``` + +--- + +### M9 — GRC20Factory Faucet: unlimited mint + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/demo/defi/grc20factory/grc20factory.gno` | +| **Lines** | 101–109 (`Faucet`) | +| **Severity** | MEDIUM | +| **Category** | Unauthorized Mint | +| **Confidence** | 0.80 | + +**Description:** + +`Faucet()` has no per-address cooldown, no global cap, and no payment requirement. Anyone can call `Faucet` in a loop across separate transactions to mint an unlimited number of tokens on any token with `faucet > 0`. The code itself contains `// FIXME: add limits?` and `// FIXME: add payment in gnot?`. If this token is used in a DeFi pool (e.g. atomicswap), the attacker can drain it by offering infinitely minted tokens. + +**Fix:** + +Implement at minimum a per-address cooldown based on block number: + +```go +if inst.faucetLastBlock.Has(caller.String()) { + lastBlock := inst.faucetLastBlock.Get(caller.String()).(int64) + if std.ChainHeight()-lastBlock < faucetCooldownBlocks { + panic("faucet: cooldown not expired") + } +} +inst.faucetLastBlock.Set(caller.String(), std.ChainHeight()) +``` + +--- + +### M10 — Events: `/admin` route accessible without authentication + +| Field | Value | +|-------|-------| +| **File** | `examples/gno.land/r/devrels/events/render.gno` | +| **Lines** | 126–133 (`Render`) | +| **Severity** | MEDIUM | +| **Category** | Information Disclosure | +| **Confidence** | 0.80 | + +**Description:** + +`Render("admin")` displays EDIT/DELETE buttons for all events without any identity check. The actual operations (`EditEvent`, `DeleteEvent`) are protected by `Ownable.AssertOwnedBy`, so they are not directly exploitable. However, the admin interface and internal event IDs are publicly visible to anyone who knows the `/admin` URL. + +**Fix:** + +Remove the `admin` path from `Render` (a view function with no `cur realm`, making authentication impossible). Provide a separate admin interface if needed. + +--- + +### M11 — Boards2/Blog: content injection via unfiltered markdown in thread bodies + +| Field | Value | +|-------|-------| +| **Files** | `examples/gno.land/r/gnoland/boards2/v1/public.gno`, `examples/gno.land/r/gnoland/blog/admin.gno` | +| **Lines** | 226–262 (boards2), 70–83 (blog) | +| **Severity** | MEDIUM | +| **Category** | Content Injection | +| **Confidence** | 0.80 | + +**Description:** + +The `reDeniedReplyLinePrefixes` filter (blocking `#`, `---`, `>`, `gno-form`) applies to **replies** only. **Thread bodies** do not pass through `assertReplyBodyIsValid`. An invited guest can create a thread containing `` tags or complex unfiltered markdown, potentially rendering a phishing form in Gnoweb. On the blog side, the `slug`, `authors`, and `tags` fields are passed without sanitization. + +**Fix:** + +Apply the same controls as `assertReplyBodyIsValid` to `assertThreadBodyIsValid`, including the `gno-form` check. For the blog, restrict the character set of `slug`, `authors`, and `tags` fields. + +--- + +## Executive Summary + +| ID | File | Severity | Category | Confidence | +|----|------|----------|----------|------------| +| H1 | `r/gnoland/blog/admin.gno:38-45` | **HIGH** | Privilege Escalation | 0.99 | +| H2 | `r/gov/dao/proxy.gno:168-178` | **HIGH** | Governance Takeover | 0.95 | +| H3 | `r/gov/dao/v3/impl/types.gno:88-118` | **HIGH** | Governance Manipulation | 0.88 | +| H4 | `r/demo/defi/atomicswap/atomicswap.gno:109-133` | **HIGH** | Fund Lock | 0.85 | +| H5 | `r/gov/dao/v3/impl/prop_requests.gno:76-90` | **HIGH** | Governance Manipulation | 0.85 | +| H6 | `r/gnoland/ghverify/contract.gno:66-67` | **HIGH** | Identity Hijacking | 0.85 | +| H7 | `r/demo/profile/profile.gno:72-115` | **HIGH** | Profile Spoofing | 0.82 | +| M1 | `r/gov/dao/v3/treasury/treasury.gno:60-86` | MEDIUM | Access Control | 0.90 | +| M2 | `p/nt/commondao/v0/record.gno:100-113` | MEDIUM | Governance Manipulation | 0.85 | +| M3 | `p/moul/once/once.gno:32-39` | MEDIUM | Invariant Break | 0.85 | +| M4 | `r/sys/users/store.gno:137-159` | MEDIUM | Access Control | 0.83 | +| M5 | `r/gnoland/blog/admin.gno:26-29` | MEDIUM | Ownership Hijacking | 0.82 | +| M6 | `r/gov/dao/v3/impl/prop_requests.gno:119-143` | MEDIUM | Governance Manipulation | 0.82 | +| M7 | `r/demo/disperse/util.gno:36-51` | MEDIUM | Accounting Error | 0.82 | +| M8 | `r/demo/profile/profile.gno:72-85` | MEDIUM | Data Integrity | 0.80 | +| M9 | `r/demo/defi/grc20factory/grc20factory.gno:101-109` | MEDIUM | Unauthorized Mint | 0.80 | +| M10 | `r/devrels/events/render.gno:126-133` | MEDIUM | Information Disclosure | 0.80 | +| M11 | `r/gnoland/boards2/v1/public.gno:226-262` | MEDIUM | Content Injection | 0.80 | + +### Mainnet priority (urgency order) + +1. **H1** — Trivial fix (`.Remove` instead of `.Set(false)`), maximum impact, no regression risk. +2. **H2 + M1** — Same root cause (`InAllowedDAOs` empty-list bypass). One fix covers both governance and treasury. +3. **H3** — Snapshotting voting power is a fundamental fix for governance integrity. +4. **H6** — On-chain identity is a core trust pillar for mainnet. +5. **H4** — Critical if atomicswap is deployed on mainnet; otherwise can be quarantined. +6. **H5, H7** — Important but exploitable only under specific conditions. diff --git a/tests/samourai-crew/README.md b/tests/samourai-crew/README.md index e79b8ab..ea67931 100644 --- a/tests/samourai-crew/README.md +++ b/tests/samourai-crew/README.md @@ -1,68 +1,287 @@ -# misc/e2e — End-to-End Test Suite +# tests/samourai-crew — End-to-End Security Test Suite -Docker-based E2E test suite for gnoland. Tests run against a single local -validator node and are executed automatically by `make test`. +Docker-based test suite targeting live gnoland networks (testnets, mainnet). +Scripts run inside a container via `gnokey` against a single remote RPC endpoint. + +## Table of contents + +- [Running](#running) +- [Structure](#structure) +- [Wallets and funding](#wallets-and-funding) +- [Shared config](#shared-config-auditcommonsh) +- [Audit — GnoVM fix regression tests](#gnovm-fix-regression-audits) +- [Audit — Application security](#application-security-audits-open-bugs) +- [E2E tests](#e2e-scripts-e2e) +- [Markdown injection tests](#security-markdown-scripts-security-markdown) +- [Stress tests](#stress-scripts-stress) +- [Exit code conventions](#exit-code-conventions) + +--- ## Running +### Via the root Makefile (standard way) + +```sh +# From the repo root +make tests-one-shot REMOTE=https://rpc.test-13-aeddi-1.gnoland.network CHAINID=test-13 +make tests-repeatable REMOTE=https://rpc.test-13-aeddi-1.gnoland.network CHAINID=test-13 +``` + +### Via the contributor Makefile + ```sh -cd misc/e2e -make test # build images, start node, run all tests -make clean # tear down containers and volumes -make logs # stream container logs +cd tests/samourai-crew +make tests-one-shot REMOTE=https://rpc.test-13-aeddi-1.gnoland.network CHAINID=test-13 +make tests-repeatable REMOTE=https://rpc.test-13-aeddi-1.gnoland.network CHAINID=test-13 ``` +### Running a single script locally (development) + +```sh +cd tests/samourai-crew + +export REMOTE=https://rpc.test-13-aeddi-1.gnoland.network +export CHAINID=test-13 +export KEY=runner PASSWORD=runner1234 GNOKEY_HOME=/tmp/gnokey +export KEY_ADDR=g1hzlg063fqrq4gltql992ssjc0xzau89t5jp63w + +# Import the runner key (once) +echo "$PASSWORD" | gnokey add --recover --insecure-password-stdin \ + --home "$GNOKEY_HOME" runner <<< \ + "chair require about ask exhaust you already finger shop turn glory spare \ + credit april rose sniff whale news economy birth table trim raccoon grit" + +./audit/audit_blog_revocation.sh +``` + +--- + ## Structure ``` -misc/e2e/ -├── run_tests.sh # main entrypoint called by docker-compose -├── docker-compose.yml # spins up gnoland + gnokey-test containers +tests/samourai-crew/ +├── Makefile — 4 required rules + Docker build +├── Dockerfile — gnokey image with test mnemonics baked in +├── run_tests.sh — orchestrator (called inside the container) +├── README.md ├── audit/ -│ ├── common.sh # shared config: RPC, chainid, key setup -│ └── audit_*.sh # one script per gnovm fix (see below) -└── e2e/ - └── e2e_*.sh # end-to-end transaction and consensus tests +│ ├── common.sh — shared config (RPC, chainid, key) +│ ├── audit_runtime_pkg.sh — GnoVM fix (2026-05-22) +│ ├── audit_chan_type.sh — GnoVM fix (2026-05-22) +│ ├── audit_security.sh — GnoVM fix (2026-05-22) +│ ├── audit_gas_alloc.sh — GnoVM fix (2026-05-22) +│ ├── audit_byteslice.sh — GnoVM fix (2026-05-22) +│ ├── audit_array_alias.sh — GnoVM fix (2026-05-22) +│ ├── audit_var_init_order.sh — GnoVM fix (2026-05-22) +│ ├── audit_cross_realm_recover.sh — GnoVM fix (2026-05-22) +│ ├── audit_nil_realm_hole.sh — GnoVM fix (2026-06-02) +│ ├── audit_launder_pointer_write.sh — GnoVM fix (2026-06-02) +│ ├── audit_launder_panic_recover.sh — GnoVM fix (2026-06-02) +│ ├── audit_cross_realm_p_arithmetic.sh — GnoVM fix (2026-06-02) +│ ├── audit_preprocess_alloc_caps.sh — GnoVM fix (2026-06-02) +│ ├── audit_panic_log_dos.sh — GnoVM fix (2026-06-02) +│ ├── audit_map_key_gas.sh — GnoVM fix (2026-06-02) +│ ├── audit_nil_func_call.sh — GnoVM fix (2026-06-02) +│ ├── audit_type_assert_nil.sh — GnoVM fix (2026-06-02) +│ ├── audit_blog_revocation.sh — App security H1 (2026-06-02) +│ ├── audit_profile_realm_spoof.sh — App security H7 (2026-06-02) +│ └── audit_profile_arbitrary_field.sh — App security M8 (2026-06-02) +├── e2e/ +│ ├── e2e_counter.sh — (2026-05-22) +│ ├── e2e_mempool_stress.sh — (2026-05-22) +│ ├── e2e_nonce_replay.sh — repeatable (2026-05-22) +│ ├── e2e_access_control.sh — (2026-06-02) +│ ├── e2e_cross_realm_callback.sh — (2026-06-02) +│ └── e2e_storage_metering.sh — (2026-06-02) +├── security-markdown/ +│ ├── common.sh +│ ├── audit_md_title_leak.sh — (2026-05-26) +│ ├── audit_md_html_inject.sh — (2026-05-26) +│ ├── audit_md_link_hijack.sh — (2026-05-26) +│ ├── audit_md_blockquote.sh — (2026-05-26) +│ └── audit_md_image_tracking.sh — (2026-05-26) +├── stress/ +│ ├── common.sh +│ ├── sybil_chaos.sh — (2026-05-22) +│ ├── sybil_precision.sh — (2026-05-22) +│ ├── sybil_salted_chaos.sh — (2026-05-22) +│ ├── sybil_oog_spam.sh — (2026-06-02) +│ └── sybil_panic_spam.sh — (2026-06-02) +└── realms/ + └── counter/ — shared realm source (used by e2e_counter) ``` -## Audit scripts (`audit/`) +--- -Each script targets a specific gnovm bugfix and verifies it is present in -the binary. Scripts exit 0 on ✅ PATCHED and exit 1 on ❌ VULNERABLE. +## Wallets and funding -| Script | Fix | What it verifies | +Three wallets are baked into the Dockerfile. They hold testnet keys with no real +value. Wallet 1 is both the main runner and the first sybil wallet. + +| Variable | Address | Role | | --- | --- | --- | -| `audit_runtime_pkg.sh` | `afd7e4808` | `runtime` import rejected in production VM | -| `audit_chan_type.sh` | `4bcd9828e` | `chan` type rejected at preprocess, not at runtime | -| `audit_security.sh` | `6a6fc4c71` + `3be0408f0` | uint64 overflow caught at compile time; infinite recursion stopped by gas limit | -| `audit_gas_alloc.sh` | `5d5f9213f` | large allocations consume gas proportionally (per-byte model) | -| `audit_byteslice.sh` | `a3a356e71` | byte-slice index mutations persist across transactions | -| `audit_array_alias.sh` | `c64feef1d` | array copy produces independent memory (no pointer aliasing) | -| `audit_var_init_order.sh` | `50ee56e64` | package-level vars initialized in dependency order | -| `audit_cross_realm_recover.sh` | `f87249327` | full state rollback when a realm panics and recover() is called | +| `ADDR_1` / `RUNNER_ADDR` | `g1hzlg063fqrq4gltql992ssjc0xzau89t5jp63w` | Deployer, main signer, stress_1 | +| `ADDR_2` | `g174tsxfpf8zj8h3tyrz4ld690xvhcjnquls6ffc` | Secondary signer (multi-wallet e2e) | +| `ADDR_3` | `g19xnaenyhe88emmge4726ta43lp3n237vvuzc2n` | Third sybil wallet | -## E2E scripts (`e2e/`) +Funding requested from the root funder before each run: -| Script | What it verifies | -| --- | --- | -| `e2e_nonce_replay.sh` | Replaying a tx with an already-consumed sequence number is rejected | -| `e2e_counter.sh` | Deploy a realm, increment state, verify committed value | -| `e2e_mempool_stress.sh` | 10 sequential txs accepted without error; final state matches expected count | +| Mode | ADDR_1 | ADDR_2 | ADDR_3 | +| --- | --- | --- | --- | +| one-shot | 150 000 000 ugnot | 15 000 000 ugnot | 15 000 000 ugnot | +| repeatable | 10 000 000 ugnot | — | — | + +--- ## Shared config (`audit/common.sh`) -All scripts source `audit/common.sh` which sets: +All scripts source `audit/common.sh`. Values are injected by `run_tests.sh` +before any script is called; the defaults below are for standalone use only. | Variable | Default | Description | | --- | --- | --- | -| `RPC` | `http://gnoland:26657` | Node RPC endpoint | +| `RPC` | `http://127.0.0.1:26657` | Node RPC endpoint | | `CHAINID` | `test` | Chain ID | -| `KEY` | `test1` | Gnokey account name | -| `PASSWORD` | `test1234` | Key password | +| `KEY` | `runner` | Gnokey account name | +| `PASSWORD` | `runner1234` | Key password | | `GNOKEY_HOME` | `/tmp/gnokey` | Gnokey home directory | -| `KEY_ADDR` | `g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5` | Deterministic address for `test1` | +| `KEY_ADDR` | `g1hzlg063fq...` | Address of the runner key | -Override any variable via environment: -```sh -RPC=http://localhost:26657 CHAINID=test-13 ./audit/audit_security.sh -``` +--- + +## GnoVM fix regression audits + +Each script targets a specific commit merged in gnolang/gno and verifies the fix +is present on the target network. +Exit 0 = ✅ PATCHED — exit 1 = ❌ VULNERABLE. + +| Script | Added | Commit | What it verifies | +| --- | --- | --- | --- | +| `audit_runtime_pkg.sh` | 2026-05-22 | `afd7e4808` | `runtime` import rejected in production VM | +| `audit_chan_type.sh` | 2026-05-22 | `4bcd9828e` | `chan` type rejected at preprocess, not at runtime | +| `audit_security.sh` | 2026-05-22 | `6a6fc4c71` + `3be0408f0` | uint64 overflow caught at compile time; infinite recursion capped by gas | +| `audit_gas_alloc.sh` | 2026-05-22 | `5d5f9213f` | large allocations consume gas proportionally (per-byte model) | +| `audit_byteslice.sh` | 2026-05-22 | `a3a356e71` | byte-slice index mutations persist across transactions | +| `audit_array_alias.sh` | 2026-05-22 | `c64feef1d` | array copy produces independent memory (no pointer aliasing) | +| `audit_var_init_order.sh` | 2026-05-22 | `50ee56e64` | package-level vars initialized in dependency order | +| `audit_cross_realm_recover.sh` | 2026-05-22 | `f87249327` | full state rollback when a realm panics and recover() is called | +| `audit_nil_realm_hole.sh` | 2026-06-02 | `2c7f1abe3` | nil-realm cross-realm write hole closed for `/p/` and stdlib | +| `audit_launder_pointer_write.sh` | 2026-06-02 | `2c7f1abe3` | pointer write via `/p/` intermediary is blocked | +| `audit_launder_panic_recover.sh` | 2026-06-02 | `f87249327` + `2c7f1abe3` | panic/recover chain via `/p/` cannot corrupt cross-realm state | +| `audit_cross_realm_p_arithmetic.sh` | 2026-06-02 | `9e56b0c77` | cross-realm arithmetic on `/p/` types produces correct results | +| `audit_preprocess_alloc_caps.sh` | 2026-06-02 | `c98a2cdca` | per-tx allocator caps large allocation attempts | +| `audit_panic_log_dos.sh` | 2026-06-02 | `4bb497abe` | panic-log rendering is bounded, prevents unmetered long txs | +| `audit_map_key_gas.sh` | 2026-06-02 | `720af8bcd` | map key operations consume gas proportionally to key complexity | +| `audit_nil_func_call.sh` | 2026-06-02 | `a7e4c34b0` | nil function call produces a proper gno panic, not a node crash | +| `audit_type_assert_nil.sh` | 2026-06-02 | `6dad8e39d` | unsafe type assertion on nil produces a proper gno panic | + +--- + +## Application security audits (open bugs) + +These scripts document **application-level security bugs** found in +`examples/gno.land/` during a manual source-code audit on 2026-06-02. Unlike +the GnoVM audits above, these bugs have not been fixed yet — no issue or open PR +existed at the time of discovery. Scripts exit 1 with `[NEW]` while the bugs +remain unreported upstream, and automatically flip to `[PASS]` once the fixes land. + +The full audit report (7 HIGH, 11 MEDIUM) is at +[`AUDIT_SECURITY_2026-06-02.md`](AUDIT_SECURITY_2026-06-02.md). This suite covers the 3 findings +testable without the GovDAO T1 multisig keys. + +### How these bugs were found + +A manual review of `examples/gno.land/r/` and `examples/gno.land/p/` was +conducted, focused on access control, state integrity, and caller-identity +patterns introduced by the interrealm Phase 3 changes (`1bed667a3`). The bugs +were identified by reading source code. None had publicly filed issues or open +PRs at the time of the audit. + +### Test details + +| Script | Added | ID | Severity | Root cause | +| --- | --- | --- | --- | --- | +| `audit_blog_revocation.sh` | 2026-06-02 | H1 | HIGH | `admin.gno:43` calls `BPTree.Set(addr, false)` instead of `BPTree.Remove(addr)`. `isModerator()` only checks `found` from `Get()`, which stays `true` even when the stored value is `false`. A revoked moderator retains full rights indefinitely. The code itself has `// FIXME: delete instead?` at the offending line. | +| `audit_profile_realm_spoof.sh` | 2026-06-02 | H7 | HIGH | `profile.gno:72-115` — `SetStringField`, `SetIntField`, `SetBoolField` use `cur.Previous().Address()` without checking `cur.Previous().IsUserCall()`. Any realm can write a profile entry under its own realm address, spoofing identity in front-ends that display profiles for known realm addresses. | +| `audit_profile_arbitrary_field.sh` | 2026-06-02 | M8 | MEDIUM | `profile.gno:72` — `SetStringField` writes any `field` string to the store without validating it against the `stringFields` allowlist. Callers can persist arbitrary key-value pairs (e.g. `"Admin": "true"`) that may mislead front-ends. | + +### How each test works + +**`audit_blog_revocation.sh`** — Deploys a minimal realm replica reproducing the +exact BPTree pattern from `r/gnoland/blog/admin.gno`. The production blog cannot +be used since its admin key is the GovDAO T1 multisig. The script adds the runner +as moderator, removes it (triggering the `Set(false)` bug), then calls the +moderator-only function. If it succeeds (`OK!`) the bug is confirmed. + +**`audit_profile_realm_spoof.sh`** — Deploys an intermediary realm +(`testprofilecaller`) that cross-calls `r/demo/profile.SetStringField`. Because +the setter reads `cur.Previous().Address()` without an `IsUserCall()` check, the +profile entry is stored under the intermediary realm's address rather than the +signing EOA. The script retrieves the realm's own address via `cur.Address()`, +then queries the profile store to confirm the mismatch. + +**`audit_profile_arbitrary_field.sh`** — Directly calls +`r/demo/profile.SetStringField` with field name `HackerField`, which does not +exist in the `stringFields` allowlist. The call succeeding without panic confirms +no field validation is enforced. No realm deployment needed. + +--- + +## E2E scripts (`e2e/`) + +Behavioral and consensus tests. All are one-shot except `e2e_nonce_replay`. + +| Script | Added | Mode | What it verifies | +| --- | --- | --- | --- | +| `e2e_counter.sh` | 2026-05-22 | one-shot | Deploy a realm, increment state, verify committed value | +| `e2e_mempool_stress.sh` | 2026-05-22 | one-shot | 10 sequential txs accepted; final state matches expected count | +| `e2e_nonce_replay.sh` | 2026-05-22 | repeatable | Replaying a tx with an already-consumed sequence is rejected | +| `e2e_access_control.sh` | 2026-06-02 | one-shot | Admin-only function rejects unauthorized callers after privilege is revoked | +| `e2e_cross_realm_callback.sh` | 2026-06-02 | one-shot | Cross-realm callback cannot modify the caller realm's state | +| `e2e_storage_metering.sh` | 2026-06-02 | one-shot | Persistent storage consumes gas proportionally (large write OOGs, small write succeeds) | + +--- + +## Security Markdown scripts (`security-markdown/`) + +Tests documenting unsafe markdown patterns in `Render()` functions. PR #5714 +(merged 2026-05-28) introduced `p/nt/markdown/sanitize/v0` as an opt-in library. +These scripts deploy intentionally naive realms that omit the sanitize library, +documenting the risk pattern. They exit 0 with `[KNOWN_VULNERABLE]` — the unsafe +pattern is intentional in the test realms, no automatic flip to PASS expected. + +| Script | Added | Vector | +| --- | --- | --- | +| `audit_md_title_leak.sh` | 2026-05-26 | Title embedded into body without `sanitize.InlineText()` → heading injection | +| `audit_md_html_inject.sh` | 2026-05-26 | Raw HTML in body without `sanitize.Block()` → HTML injection | +| `audit_md_link_hijack.sh` | 2026-05-26 | User-controlled URL without `sanitize.URL()` → link hijacking | +| `audit_md_blockquote.sh` | 2026-05-26 | Blockquote content without `sanitize.Block()` → content injection | +| `audit_md_image_tracking.sh` | 2026-05-26 | Image URL without `sanitize.ImageURL()` → tracking pixel injection | + +--- + +## Stress scripts (`stress/`) + +Parallel sybil tests using 3 wallets sending transactions concurrently against a +single RPC endpoint. Each test verifies the node remains stable and responsive +after the load. + +| Script | Added | What it verifies | +| --- | --- | --- | +| `sybil_chaos.sh` | 2026-05-22 | N wallets × M parallel txs — node accepts all valid txs, final state consistent | +| `sybil_precision.sh` | 2026-05-22 | Concurrent increments from N wallets — final count matches N×M exactly | +| `sybil_salted_chaos.sh` | 2026-05-22 | Salted random txs from N wallets in parallel — no sequence collision | +| `sybil_oog_spam.sh` | 2026-06-02 | N wallets × M under-gas txs — all rejected with OOG, node still responsive | +| `sybil_panic_spam.sh` | 2026-06-02 | N wallets × M panic-triggering txs — all rejected cleanly, node still responsive | + +--- + +## Exit code conventions + +| Exit | Summary label | Script output | Meaning | +| --- | --- | --- | --- | +| 0 | `[PASS]` | `[PASS]` | Expected behavior confirmed (fix present, or test passed) | +| 1 | `[NEW]` | `[KNOWN_VULNERABLE]` | Bug confirmed, not yet reported upstream — no issue or PR exists | +| 1 | `[KNOWN]` | `[KNOWN_VULNERABLE]` | Bug confirmed, tracked upstream with an open issue or PR | +| 1 | `[FAIL]` | `[FAIL]` | Unexpected result — investigate | diff --git a/tests/samourai-crew/audit/audit_blog_revocation.sh b/tests/samourai-crew/audit/audit_blog_revocation.sh new file mode 100755 index 0000000..5f922e0 --- /dev/null +++ b/tests/samourai-crew/audit/audit_blog_revocation.sh @@ -0,0 +1,134 @@ +#!/bin/sh +# H1 — Blog : révocation moderator/commenter définitivement cassée +# Fichier gno.land concerné : examples/gno.land/r/gnoland/blog/admin.gno:38-45,119-127 +# +# Root cause : AdminRemoveModerator appelle BPTree.Set(addr, false) au lieu de +# BPTree.Remove(addr). isModerator() ne teste que le booléen 'found' retourné +# par Get() — found reste true même si la valeur stockée est false. +# La révocation est donc sans effet. +# +# Ce test déploie un realm replica qui reproduit fidèlement le bug, puis vérifie +# que l'ex-modérateur peut toujours appeler la fonction protégée. +# +# Résultat attendu (bug présent) : [KNOWN_VULNERABLE] +# Résultat attendu (bug corrigé) : [PASS] + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +# shellcheck source=common.sh +. "$SCRIPT_DIR/common.sh" + +SUFFIX=$(date +%s) +PKGPATH="gno.land/r/${KEY_ADDR}/audit/testblog${SUFFIX}" +TMPDIR=$(mktemp -d) +trap 'rm -rf "$TMPDIR"' EXIT + +echo "H1 — Blog revocation cassée (BPTree.Set false vs Remove)" +echo " Package: $PKGPATH" + +# --- realm replica reproduisant le bug --- +cat > "$TMPDIR/testblog.gno" << EOF +package testblog${SUFFIX} + +import "gno.land/p/nt/bptree/v0" + +var moderatorList = bptree.NewBPTree32() + +// AddModerator marque addr comme modérateur. +func AddModerator(_ realm, addr string) { + moderatorList.Set(addr, true) +} + +// RemoveModerator reproduit le bug du blog : Set(false) au lieu de Remove. +func RemoveModerator(_ realm, addr string) { + moderatorList.Set(addr, false) +} + +// isModerator reproduit le bug : teste only 'found', pas la valeur. +func isModerator(addr string) bool { + _, found := moderatorList.Get(addr) + return found +} + +// ModOnlyAction panique si l'appelant n'est pas modérateur. +func ModOnlyAction(cur realm) string { + caller := cur.Previous().Address() + if !isModerator(caller.String()) { + panic("not moderator") + } + return "executed" +} + +func Render(_ string) string { return "testblog" } +EOF + +cat > "$TMPDIR/gnomod.toml" << EOF +module = "${PKGPATH}" +gno = "0.9" +EOF + +# --- déploiement --- +echo -n " Deploying testblog replica... " +DEPLOY=$(echo "$PASSWORD" | gnokey maketx addpkg \ + -pkgpath "$PKGPATH" -pkgdir "$TMPDIR" \ + -gas-fee 1000000ugnot -gas-wanted 20000000 \ + -broadcast -chainid "$CHAINID" -remote "$RPC" \ + -insecure-password-stdin \ + -home "$GNOKEY_HOME" \ + "$KEY" 2>&1) +if echo "$DEPLOY" | grep -q "OK!"; then echo "OK"; else + echo "FAILED"; echo "$DEPLOY"; exit 1 +fi + +# --- step 1 : ajouter le runner comme modérateur --- +echo -n " Step 1: AddModerator($KEY_ADDR)... " +ADD=$(echo "$PASSWORD" | gnokey maketx call \ + -pkgpath "$PKGPATH" -func "AddModerator" \ + -args "$KEY_ADDR" \ + -gas-fee 1000000ugnot -gas-wanted 10000000 \ + -broadcast -chainid "$CHAINID" -remote "$RPC" \ + -insecure-password-stdin \ + -home "$GNOKEY_HOME" \ + "$KEY" 2>&1) +if echo "$ADD" | grep -q "OK!"; then echo "OK"; else + echo "FAILED"; echo "$ADD"; exit 1 +fi + +# --- step 2 : révoquer (appelle Set(false) — le bug) --- +echo -n " Step 2: RemoveModerator($KEY_ADDR)... " +REM=$(echo "$PASSWORD" | gnokey maketx call \ + -pkgpath "$PKGPATH" -func "RemoveModerator" \ + -args "$KEY_ADDR" \ + -gas-fee 1000000ugnot -gas-wanted 10000000 \ + -broadcast -chainid "$CHAINID" -remote "$RPC" \ + -insecure-password-stdin \ + -home "$GNOKEY_HOME" \ + "$KEY" 2>&1) +if echo "$REM" | grep -q "OK!"; then echo "OK (Set to false — bug not fixed)"; else + echo "FAILED"; echo "$REM"; exit 1 +fi + +# --- step 3 : tenter ModOnlyAction en tant qu'ex-modérateur --- +echo -n " Step 3: ModOnlyAction() as revoked moderator (expect: still works = KNOWN_VULNERABLE)... " +MOD=$(echo "$PASSWORD" | gnokey maketx call \ + -pkgpath "$PKGPATH" -func "ModOnlyAction" \ + -gas-fee 1000000ugnot -gas-wanted 10000000 \ + -broadcast -chainid "$CHAINID" -remote "$RPC" \ + -insecure-password-stdin \ + -home "$GNOKEY_HOME" \ + "$KEY" 2>&1) + +if echo "$MOD" | grep -q "OK!"; then + echo "" + echo " [KNOWN_VULNERABLE] — revoked moderator can still call ModOnlyAction" + echo " Root cause: BPTree.Set(addr, false) leaves 'found=true', isModerator() returns true" + echo " Fix: use moderatorList.Remove(addr.String()) in AdminRemoveModerator" + exit 1 +elif echo "$MOD" | grep -qi "not moderator\|panic\|unauthorized"; then + echo "" + echo " [PASS] — revocation is effective, ModOnlyAction correctly rejected" +else + echo "" + echo " [FAIL] — unexpected output" + echo "$MOD" + exit 1 +fi diff --git a/tests/samourai-crew/audit/audit_profile_arbitrary_field.sh b/tests/samourai-crew/audit/audit_profile_arbitrary_field.sh new file mode 100755 index 0000000..29373ac --- /dev/null +++ b/tests/samourai-crew/audit/audit_profile_arbitrary_field.sh @@ -0,0 +1,82 @@ +#!/bin/sh +# M8 — Profile : noms de champs arbitraires acceptés +# Fichier gno.land concerné : examples/gno.land/r/demo/profile/profile.gno:72-85 +# +# Root cause : SetStringField n'effectue aucune validation du paramètre 'field' +# contre la liste des champs autorisés (stringFields map). N'importe quelle +# string est acceptée et persistée dans le store, créant des entrées arbitraires. +# +# Ce test appelle directement r/demo/profile.SetStringField avec un champ inconnu +# et vérifie que la transaction réussit (bug) ou est rejetée (fix). +# +# Aucun déploiement de realm requis — appel direct sur le realm genesis. +# +# Résultat attendu (bug présent) : [KNOWN_VULNERABLE] +# Résultat attendu (bug corrigé) : [PASS] + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +# shellcheck source=common.sh +. "$SCRIPT_DIR/common.sh" + +PROFILE_PKGPATH="gno.land/r/demo/profile" +TEST_FIELD="HackerField" +TEST_VALUE="injected_by_audit" + +echo "M8 — Profile arbitrary field name accepted" +echo " Target: $PROFILE_PKGPATH" +echo " Field: $TEST_FIELD (not in stringFields allowlist)" + +# --- vérifier que le realm profile est accessible --- +echo -n " Checking profile realm accessibility... " +PING=$(gnokey query "vm/qeval" \ + -data "${PROFILE_PKGPATH}.GetStringField(\"${KEY_ADDR}\", \"DisplayName\", \"\")" \ + -remote "$RPC" 2>&1) +if echo "$PING" | grep -qiE 'error|not found|no such'; then + echo "FAILED — r/demo/profile not accessible on this network" + echo "$PING" + exit 1 +fi +echo "OK" + +# --- appel avec champ arbitraire --- +echo -n " SetStringField(\"$TEST_FIELD\", \"$TEST_VALUE\")... " +CALL=$(echo "$PASSWORD" | gnokey maketx call \ + -pkgpath "$PROFILE_PKGPATH" -func "SetStringField" \ + -args "$TEST_FIELD" -args "$TEST_VALUE" \ + -gas-fee 1000000ugnot -gas-wanted 10000000 \ + -broadcast -chainid "$CHAINID" -remote "$RPC" \ + -insecure-password-stdin \ + -home "$GNOKEY_HOME" \ + "$KEY" 2>&1) + +if echo "$CALL" | grep -q "OK!"; then + echo "OK (accepted — bug active)" + + # vérifier que le champ est bien persisté + echo -n " Verifying field persisted in store... " + READ=$(gnokey query "vm/qeval" \ + -data "${PROFILE_PKGPATH}.GetStringField(\"${KEY_ADDR}\", \"${TEST_FIELD}\", \"\")" \ + -remote "$RPC" 2>&1) + + if echo "$READ" | grep -q "$TEST_VALUE"; then + echo "found \"$TEST_VALUE\"" + echo " [KNOWN_VULNERABLE] — arbitrary field '$TEST_FIELD' accepted and persisted" + echo " Fix: validate 'field' against stringFields allowlist before writing" + echo " if _, ok := stringFields[field]; !ok { panic(\"profile: unknown field: \" + field) }" + exit 1 + else + echo "not found (unexpected)" + echo "$READ" + echo " [FAIL] — call returned OK! but field not readable" + exit 1 + fi + +elif echo "$CALL" | grep -qi "unknown field\|invalid field\|panic"; then + echo "rejected" + echo " [PASS] — unknown field '$TEST_FIELD' correctly rejected" +else + echo "unexpected output" + echo "$CALL" + echo " [FAIL] — unexpected result from SetStringField" + exit 1 +fi diff --git a/tests/samourai-crew/audit/audit_profile_realm_spoof.sh b/tests/samourai-crew/audit/audit_profile_realm_spoof.sh new file mode 100755 index 0000000..57c0d0a --- /dev/null +++ b/tests/samourai-crew/audit/audit_profile_realm_spoof.sh @@ -0,0 +1,134 @@ +#!/bin/sh +# H7 — Profile : manque de garde IsUserCall() → tout realm peut écrire un profil +# Fichier gno.land concerné : examples/gno.land/r/demo/profile/profile.gno:72-115 +# +# Root cause : SetStringField/SetIntField/SetBoolField utilisent cur.Previous().Address() +# sans vérifier cur.Previous().IsUserCall(). Quand un realm intermédiaire appelle le +# setter, cur.Previous() est l'adresse du realm — pas l'EOA signataire. +# N'importe quel realm peut donc écrire un profil sous sa propre adresse realm. +# +# Ce test déploie un realm appelant (testprofilecaller) qui cross-call SetStringField. +# Le profil est alors stocké sous l'adresse du realm, pas sous celle du signataire EOA. +# +# Résultat attendu (bug présent) : [KNOWN_VULNERABLE] +# Résultat attendu (bug corrigé) : [PASS] + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +# shellcheck source=common.sh +. "$SCRIPT_DIR/common.sh" + +SUFFIX=$(date +%s) +PKGPATH="gno.land/r/${KEY_ADDR}/audit/testprofilecaller${SUFFIX}" +TMPDIR=$(mktemp -d) +trap 'rm -rf "$TMPDIR"' EXIT + +PROFILE_PKGPATH="gno.land/r/demo/profile" +TEST_FIELD="DisplayName" +TEST_VALUE="hacked_by_realm_${SUFFIX}" + +echo "H7 — Profile realm spoof (missing IsUserCall guard)" +echo " Caller realm: $PKGPATH" +echo " Target realm: $PROFILE_PKGPATH" + +# --- realm appelant qui cross-call profile.SetStringField --- +cat > "$TMPDIR/testprofilecaller.gno" << EOF +package testprofilecaller${SUFFIX} + +import "gno.land/r/demo/profile" + +// SpoofProfile appelle SetStringField cross-realm. +// Le profil sera stocké sous l'adresse de CE realm (pas l'EOA signataire). +func SpoofProfile(cur realm, field, value string) { + profile.SetStringField(cross(cur), field, value) +} + +// GetAddr retourne l'adresse de ce realm (pour la vérification). +func GetAddr(cur realm) string { + return cur.Address().String() +} +EOF + +cat > "$TMPDIR/gnomod.toml" << EOF +module = "${PKGPATH}" +gno = "0.9" +EOF + +# --- déploiement --- +echo -n " Deploying testprofilecaller realm... " +DEPLOY=$(echo "$PASSWORD" | gnokey maketx addpkg \ + -pkgpath "$PKGPATH" -pkgdir "$TMPDIR" \ + -gas-fee 1000000ugnot -gas-wanted 10000000 \ + -broadcast -chainid "$CHAINID" -remote "$RPC" \ + -insecure-password-stdin \ + -home "$GNOKEY_HOME" \ + "$KEY" 2>&1) +if echo "$DEPLOY" | grep -q "OK!"; then echo "OK"; else + echo "FAILED"; echo "$DEPLOY"; exit 1 +fi + +# --- récupérer l'adresse du realm déployé --- +echo -n " Getting realm address... " +REALM_ADDR_OUTPUT=$(gnokey query "vm/qeval" \ + -data "${PKGPATH}.GetAddr()" \ + -remote "$RPC" 2>&1) +REALM_ADDR=$(echo "$REALM_ADDR_OUTPUT" | grep -oE 'g1[a-z0-9]{38}' | head -1) +if [ -z "$REALM_ADDR" ]; then + echo "FAILED — could not retrieve realm address" + echo "$REALM_ADDR_OUTPUT" + exit 1 +fi +echo "$REALM_ADDR" + +# --- step 1 : appel cross-realm sur profile.SetStringField --- +echo -n " Step 1: SpoofProfile(\"$TEST_FIELD\", \"$TEST_VALUE\")... " +SPOOF=$(echo "$PASSWORD" | gnokey maketx call \ + -pkgpath "$PKGPATH" -func "SpoofProfile" \ + -args "$TEST_FIELD" -args "$TEST_VALUE" \ + -gas-fee 1000000ugnot -gas-wanted 10000000 \ + -broadcast -chainid "$CHAINID" -remote "$RPC" \ + -insecure-password-stdin \ + -home "$GNOKEY_HOME" \ + "$KEY" 2>&1) +if echo "$SPOOF" | grep -q "OK!"; then + echo "OK (cross-realm call accepted)" +elif echo "$SPOOF" | grep -qi "only direct EOA\|IsUserCall\|panic"; then + echo "rejected (fix active)" + echo " [PASS] — profile.SetStringField correctly rejects realm callers" + exit 0 +else + echo "FAILED unexpectedly"; echo "$SPOOF"; exit 1 +fi + +# --- step 2 : vérifier que le profil est stocké sous l'adresse REALM --- +echo -n " Step 2: Check profile stored under REALM address ($REALM_ADDR)... " +REALM_PROFILE=$(gnokey query "vm/qeval" \ + -data "${PROFILE_PKGPATH}.GetStringField(\"${REALM_ADDR}\", \"${TEST_FIELD}\", \"\")" \ + -remote "$RPC" 2>&1) + +if echo "$REALM_PROFILE" | grep -q "$TEST_VALUE"; then + echo "found \"$TEST_VALUE\"" + + # --- step 3 : vérifier que le profil n'est PAS sous l'EOA ($KEY_ADDR) --- + echo -n " Step 3: Check profile NOT stored under EOA address ($KEY_ADDR)... " + EOA_PROFILE=$(gnokey query "vm/qeval" \ + -data "${PROFILE_PKGPATH}.GetStringField(\"${KEY_ADDR}\", \"${TEST_FIELD}\", \"\")" \ + -remote "$RPC" 2>&1) + + if echo "$EOA_PROFILE" | grep -q "$TEST_VALUE"; then + echo "FOUND under EOA too (unexpected)" + echo " [FAIL] — profile stored under both realm and EOA addresses" + exit 1 + else + echo "not found (correct)" + echo " [KNOWN_VULNERABLE] — profile written under realm address, not EOA" + echo " REALM_ADDR : $REALM_ADDR (has profile)" + echo " EOA_ADDR : $KEY_ADDR (no profile)" + echo " Fix: add 'if !cur.Previous().IsUserCall() { panic(...) }' to each setter" + exit 1 + fi +else + echo "not found — unexpected" + echo "$REALM_PROFILE" + echo " [FAIL] — cross-realm call returned OK! but profile not found under realm address" + exit 1 +fi diff --git a/tests/samourai-crew/run_tests.sh b/tests/samourai-crew/run_tests.sh index 9776369..2d1a05f 100755 --- a/tests/samourai-crew/run_tests.sh +++ b/tests/samourai-crew/run_tests.sh @@ -144,17 +144,21 @@ fi echo "" # --- test runner --- -PASS=0; FAIL=0; KNOWN=0; REPORT="" +PASS=0; FAIL=0; KNOWN=0; NEW=0; REPORT="" run_test() { NAME="$1" SCRIPT="$2" KNOWN_NOTE="$3" + NEW_NOTE="$4" echo "" echo "--- $NAME ---" if "$SCRIPT"; then PASS=$((PASS + 1)) REPORT="${REPORT} [PASS] $NAME\n" + elif [ -n "$NEW_NOTE" ]; then + NEW=$((NEW + 1)) + REPORT="${REPORT} [NEW] $NAME — $NEW_NOTE\n" elif [ -n "$KNOWN_NOTE" ]; then KNOWN=$((KNOWN + 1)) REPORT="${REPORT} [KNOWN] $NAME — $KNOWN_NOTE\n" @@ -184,8 +188,15 @@ if [ "$MODE" = "one-shot" ] || [ "$MODE" = "all" ]; then run_test "audit_nil_realm_hole" /tests/audit/audit_nil_realm_hole.sh run_test "audit_launder_pointer_write" /tests/audit/audit_launder_pointer_write.sh run_test "audit_launder_panic_recover" /tests/audit/audit_launder_panic_recover.sh - - + + echo "" + echo "=== Security App Audit (realm application bugs) ===" + run_test "audit_blog_revocation" /tests/audit/audit_blog_revocation.sh \ + "" "H1: blog revocation broken (BPTree.Set vs Remove) — unreported, see AUDIT_SECURITY_2026-06-02.md" + run_test "audit_profile_realm_spoof" /tests/audit/audit_profile_realm_spoof.sh \ + "" "H7: profile missing IsUserCall guard — unreported, see AUDIT_SECURITY_2026-06-02.md" + run_test "audit_profile_arbitrary_field" /tests/audit/audit_profile_arbitrary_field.sh \ + "" "M8: profile arbitrary field names accepted — unreported, see AUDIT_SECURITY_2026-06-02.md" echo "" echo "=== E2E Tests (one-shot) ===" @@ -229,7 +240,7 @@ echo " TEST SUMMARY" echo "=========================================" printf "%b" "$REPORT" echo "-----------------------------------------" -echo " PASS: $PASS FAIL: $FAIL KNOWN: $KNOWN" +echo " PASS: $PASS FAIL: $FAIL KNOWN: $KNOWN NEW: $NEW" echo "=========================================" if [ "$FAIL" -gt 0 ]; then