Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,13 @@ make fmt # Format all code
## Gno Security Semantics

- Before writing or reviewing any caller-authentication, access-control, or cross-realm code in Gno (`/r/`, `/p/`, `/e/` packages), read `docs/resources/gno-interrealm.md`. Do not pattern-match from Solidity `msg.sender` or other-language intuition.
- For the complete AI-focused checklist (10 cases), read `docs/resources/gno-ai-contract-review.md` before reviewing any realm.
- `runtime.PreviousRealm()` only shifts on explicit cross-calls (`fn(cross, ...)`) into crossing functions (`func fn(cur realm, ...){...}`). A `PreviousRealm().PkgPath() == "..."` check inside a non-crossing function does not identify the immediate caller and is a security bug.
- When editing a realm that accepts payment via `banker.OriginSend()`, the caller guard must be `runtime.PreviousRealm().IsUserCall()`, not `IsUser()`. `IsUser()` accepts `maketx run` ephemeral realms, which can consume the origin-send envelope before calling your function and bypass the payment check. See [docs/resources/effective-gno.md § Verifying inbound Coin payments](docs/resources/effective-gno.md#verifying-inbound-coin-payments).
- In crossing functions (`func F(cur realm, ...)`), **always** use `cur.IsCurrent()` before calling `cur.Previous()`. Never use `chain/runtime/unsafe.PreviousRealm()` in a crossing function — it bypasses the frame verification that `cur.IsCurrent()` provides. Any import of `chain/runtime/unsafe` alongside `cur realm` parameters is a red flag.
- When editing a realm that accepts payment via `banker.OriginSend()`, the caller guard must be `cur.Previous().IsUserCall()`, not `IsUser()`. `IsUser()` accepts `maketx run` ephemeral realms, which can consume the origin-send envelope before calling your function and bypass the payment check. See [docs/resources/effective-gno.md § Verifying inbound Coin payments](docs/resources/effective-gno.md#verifying-inbound-coin-payments).
- When you see an existing realm using `IsUser()` plus `banker.OriginSend()`, flag it and cross-check nearby `OriginSend` usage.
- Never return a pointer to a `/p/`-type instance stored in realm state if that type has any exported mutation method (e.g. `avl.Tree.Set`, `avl.Tree.Remove`). Readonly taint does not block method dispatch — borrow rule #2 fires and the write commits under your realm's authority. Return values or narrow read-only views instead.
- `Render(path string)` receives attacker-controlled input. Never write path segments, user-supplied keys, or free-form string values directly into markdown output. Use `sanitize.InlineText` from `gno.land/p/nt/markdown/sanitize/v0` for inline content.

---

Expand Down
213 changes: 213 additions & 0 deletions docs/resources/gno-ai-contract-review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
# Gno Contract Review Guide for AI Agents

A concise reference for AI agents performing security review of `.gno` realm code.
For the full threat model and worked examples, see [`gno-security-guide.md`](./gno-security-guide.md).

---

## Quick Checks

These are the highest-yield issues to look for in any realm:

### 1. Caller identity — use `cur realm`, not `address` parameters

```go
// WRONG: address parameter is attacker-controlled
func AdminAction(caller address) { ... }

// RIGHT: derive identity from the live crossing frame
func AdminAction(cur realm) {
if !cur.IsCurrent() { panic("spoofed realm") }
addr := cur.Previous().Address()
...
}
```

### 2. Payment guards — `IsUserCall()`, not `IsUser()`

```go
// WRONG: MsgRun ephemeral realms pass IsUser()
if !IsUser() { panic("not a user") }

// RIGHT
if !cur.Previous().IsUserCall() { panic("not a direct user call") }
```

### 3. No exported pointers to mutable state

```go
// WRONG: attacker can call mutator methods on returned pointer
func GetAccount() *Account { return gAccount }

// RIGHT: return a copy, or expose read-only accessors
func GetBalance() int { return gAccount.balance }
```

### 4. No caller-supplied callbacks invoked with realm authority

```go
// WRONG: if fn is a top-level /p/-declared function, it inherits
// the caller's m.Realm and can write to your state
func ApplyHook(fn func()) { fn() }

// RIGHT: type the callback with your own /r/-declared type so
// /p/ code can't supply a matching implementation
func ApplyHook(fn func(*MyState)) { fn(gState) }
```

### 5. Interface parameters need canonical-type assertion

```go
// WRONG: Evil{Teller} embedding bypasses interface checks
func DoBanking(t grc20.Teller) { t.Transfer(...) }

// RIGHT: assert the concrete type before dispatch
func DoBanking(t grc20.Teller) {
if !grc20.IsCanonicalTeller(t) { panic("not a canonical Teller") }
t.Transfer(...)
}
```

### 6. Do not store `realm` values

`realm` values are ephemeral — store `Address()` or `PkgPath()` strings instead.

```go
// WRONG: panics at attach time
var savedRealm realm

// RIGHT
var savedAddr address
func Save(cur realm) { savedAddr = cur.Previous().Address() }
```

### 7. `/p/`-type with callback iterators

If a realm field is a `/p/`-type with methods like `Iterate(cb func(*Node) bool)`,
attackers can supply a top-level `/p/`-function that runs with your realm's authority.
Keep such fields unexported **and** do not return aliased pointers to them.

### 8. `/p/`-type with mutation methods returned as pointer

This is the subtlest case. A `/p/` library type whose fields are all **unexported** can
still be a write-authority leak if it has exported mutation methods and you return a pointer
to an instance stored in your realm.

```go
// avl.Tree fields are all unexported — looks safe.
// But Tree has exported mutation methods: Set, Remove, etc.
var store = avl.NewTree()

// WRONG: attacker calls store.Set(key, value) on the returned pointer.
// Borrow rule #2 fires (tree was allocated in this realm) → m.Realm = /r/V
// for the method body → the write inside Set commits under your authority.
func GetStore() *avl.Tree { return store }

// RIGHT: never return the tree pointer. Expose only what you control.
func GetValue(key string) (any, bool) { return store.Get(key) }
```

The rule: **any exported method on a `/p/` type that writes to its receiver is a
mutator**. If you return a pointer to an instance, that mutator is now callable by
anyone with the authority of your realm.

#### Sub-case: exported pointer fields

The same path exists through exported pointer fields of `/p/` structs:

```go
// p/mylib
type Container struct {
Items *avl.Tree // exported pointer field
}

// r/V
var c = &Container{Items: avl.NewTree()}
func GetContainer() *Container { return c }

// Attacker: c.Items.Set(key, value) → borrow rule #2 on Items
// (Items was allocated in /r/V) → write commits.
```

Readonly taint on `c` does NOT block this: method dispatch is not a write operation,
so the taint check does not fire. Borrow rule #2 fires first on method entry and
authorizes the writes inside the method body.

**Rule**: treat every exported pointer field of a `/p/` type as if it were a direct
pointer to mutable state. If the pointed-to type has any mutation method, it is a
live mutator handle. Never return the containing struct as a pointer.

### 9. `unsafe.PreviousRealm()` — old API, skips frame verification

Using `chain/runtime/unsafe.PreviousRealm()` directly bypasses the `cur.IsCurrent()`
safety check. It should never appear alongside a `cur realm` parameter.

```go
// WRONG: cur is accepted but ignored; no IsCurrent() guard
import "chain/runtime/unsafe"
func Set(cur realm, key, value string) {
caller := unsafe.PreviousRealm().Address()
...
}

// RIGHT
func Set(cur realm, key, value string) {
if !cur.IsCurrent() { panic("spoofed realm") }
caller := cur.Previous().Address()
...
}
```

Flag any import of `chain/runtime/unsafe` in a realm that also has `cur realm` parameters.

### 10. Unsanitized user input in `Render`

`Render(path string)` receives attacker-controlled input. Writing path segments, keys,
or user-supplied values directly into markdown output enables injection (broken table
cells, injected links, heading overrides).

```go
// WRONG: path, keys, and values written raw
func Render(path string) string {
return "# Vault: " + path + "\n" // heading injection
}

// ALSO WRONG: table cell content not escaped
b.WriteString("| " + key + " | " + val + " |\n") // | in key breaks table

// RIGHT: escape pipe characters at minimum; use sanitize.InlineText for
// full inline markdown escaping
import "gno.land/p/nt/markdown/sanitize/v0"
b.WriteString("| " + sanitize.InlineText(key) + " | " + sanitize.InlineText(val) + " |\n")
```

---

## Review Checklist

- [ ] Authenticated mutators take `cur realm` and call `cur.IsCurrent()`
- [ ] No import of `chain/runtime/unsafe` alongside `cur realm` parameters
- [ ] Payment-guarded functions use `cur.Previous().IsUserCall()`
- [ ] No exported function returns a pointer to internal mutable state
- [ ] No exported function returns a `/p/`-type pointer whose type has mutation methods
- [ ] No exported `/p/`-struct field is itself a pointer to a type with mutation methods
- [ ] No method accepts a `func(...)` callback with a `/p/`-typed parameter and invokes it
- [ ] Interface parameters from external callers are guarded with canonical-type asserts
- [ ] No `realm`-typed value in package-level vars, struct fields, or closure captures
- [ ] `/p/`-type fields with callback iterators are unexported
- [ ] Data types holding sensitive state are declared in this realm (`/r/`), not in shared `/p/`
- [ ] `Render` sanitizes path segments, keys, and user-supplied values before writing to output

---

## Relationship to Other Docs

| Resource | Purpose |
|----------|---------|
| [`gno-security-guide.md`](./gno-security-guide.md) | Deep technical explanation of the threat model, borrow rules, and anti-patterns |
| [`gno-security.md`](./gno-security.md) | Numbered threat-class taxonomy |
| [`gno-interrealm.md`](./gno-interrealm.md) | Cross-realm call mechanics (`cur realm`, `IsCurrent()`, borrow rules) |
| [`effective-gno.md`](./effective-gno.md) | Idiomatic Gno patterns including payment guards |
| `misc/audit-pattern-harness/` | Automated pattern detection tooling with sanitized fixtures |

This guide distills the above into the shortest checklist that catches the most critical issues.
85 changes: 85 additions & 0 deletions docs/resources/gno-security-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,57 @@ back to `/r/V`, and the write commits.
**Rule**: getters return either values (copies), unexported method
results, or read-only views. Never a pointer to internal mutable state.

#### 5.1a `/p/`-type with unexported fields but exported mutation methods

The taint-bypasses-method-dispatch gap applies even when the `/p/`-type
has *no exported fields at all*. If every field is unexported but the
type has exported methods that mutate the receiver, returning a pointer
to an instance stored in your realm is equivalent to publishing those
mutators as your own API.

`avl.Tree` is the canonical example. All fields (`root`, `size`) are
unexported — a naive reviewer sees no exposed state. But `Tree.Set`,
`Tree.Remove`, and `Tree.ReverseIterate` all mutate or traverse the
tree. An attacker who receives a `*avl.Tree` pointer can call
`tree.Set(key, value)`, borrow rule #2 fires (the tree was allocated
in `/r/V`), and the write commits under victim authority.

```go
var store = avl.NewTree()

// WRONG — all fields unexported, but exported methods are mutators
func GetStore() *avl.Tree { return store }

// Attacker: GetStore().Set("k", "injected") → commits under /r/V
```

#### 5.1b Exported pointer fields on `/p/` structs

The same path exists one level of indirection deeper. If a `/p/` struct
has an exported field that is itself a pointer type whose type has
mutation methods, returning a pointer to the containing struct gives
indirect access to that inner mutator.

```go
// p/mylib
type Container struct {
Items *avl.Tree // exported pointer field — mutation methods reachable
Label string
}

// r/V
var c = &Container{Items: avl.NewTree()}
func GetContainer() *Container { return c }

// Attacker: GetContainer().Items.Set(key, value)
// Readonly taint on c does NOT block method dispatch.
// Borrow rule #2 fires on Items (allocated in /r/V) → write commits.
```

**Rule**: treat every exported pointer field of a `/p/` type as a live
mutator handle if the pointed-to type has any mutation method. Never
return the containing struct as a pointer.

### 5.2 Embedding a `/p/`-type with concrete-callback higher-order methods

The (B)-class vector. Even if your container is `/r/`-declared, if
Expand Down Expand Up @@ -327,6 +378,40 @@ a call frame`.
**Rule**: if you need to remember a caller across transactions, store
the `Address()` or `PkgPath()` (plain strings), not the realm value.

### 5.8 `unsafe.PreviousRealm()` alongside a `cur realm` parameter

`chain/runtime/unsafe.PreviousRealm()` is the pre-`cur realm` API for
obtaining the previous realm. Using it in a crossing function that already
receives `cur realm` is always wrong: it bypasses the `IsCurrent()` frame
verification that makes `cur.Previous()` safe, and silently ignores the
`cur` capability token the runtime minted for exactly this purpose.

```go
// WRONG: cur is accepted but never used; no IsCurrent() guard
import "chain/runtime/unsafe"

func Set(cur realm, key, value string) {
caller := unsafe.PreviousRealm().Address() // skips frame check
...
}

// RIGHT
func Set(cur realm, key, value string) {
if !cur.IsCurrent() { panic("spoofed realm") }
caller := cur.Previous().Address()
...
}
```

Any import of `chain/runtime/unsafe` in a realm that also declares
crossing functions (`func F(cur realm, ...)`) is a red flag. The
`unsafe` package is appropriate only in non-crossing helpers or
in realms that have not yet been migrated to the `cur realm` API.

**Rule**: in crossing functions, always derive caller identity from
`cur.Previous()` under a `cur.IsCurrent()` guard. Delete the
`chain/runtime/unsafe` import.

---

## 6. Properties That Make the Boundary Stronger Than Expected
Expand Down