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
1 change: 1 addition & 0 deletions docs/detectors/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ detector page and to the relevant [Glossary](../glossary.md) term.
| [`tier_boundary_off_by_one`](tier_boundary_off_by_one.md) | [`S022`](../error-codes.md) | logic | Info | `if`/`else if` tier/rank ladder mixes strict and inclusive comparisons on the same variable |
| [`wrong_auth_args`](wrong_auth_args.md) | [`S024`](../error-codes.md) | authentication | Medium | Internal function uses `require_auth()` instead of `require_auth_for_args()` |
| [`reserve_withdrawal`](reserve_withdrawal.md) | [`S023`](../error-codes.md) | authorization | High | Missing strict authorization guard on reserve or treasury funds withdrawal |
| [`admin_event_missing`](admin_event_missing.md) | [`SANCT_ADMIN_EVENT_MISSING`](../error-codes.md) | events | Warning | Admin/config-change functions that mutate storage without emitting an event |

## Page anatomy

Expand Down
79 changes: 79 additions & 0 deletions docs/detectors/admin_event_missing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# `admin_event_missing` — Admin/config function mutates storage without emitting an event

| | |
| --- | --- |
| **Finding code** | [`SANCT_ADMIN_EVENT_MISSING`](../error-codes.md) |
| **Category** | events |
| **Severity** | Warning |
| **Source rule** | [`rules/admin_event_missing.rs`](../../tooling/sanctifier-core/src/rules/admin_event_missing.rs) |
| **Glossary** | [Event](../glossary.md#event) · [Persistent storage](../glossary.md#persistent-storage) |

## What it catches

A `#[contractimpl]` function whose name indicates an admin or configuration-change intent
(e.g. `set_`, `update_`, `change_`, `upgrade`, `pause`, `unpause`, `migrate`, `set_admin`,
`set_owner`, `configure`, `transfer_admin`) that performs a storage mutation —
`.set(…)`, `.update(…)`, or `.remove(…)` — **without** emitting a corresponding on-chain event.

Off-chain monitors, indexers, dashboards, and governance tools track privileged changes via
events. A silent storage write makes those changes invisible, preventing real-time alerting
and complicating incident response.

## Vulnerable example

```rust
#[contractimpl]
impl Token {
pub fn set_admin(env: Env, new_admin: Address) {
// Writes storage but never emits an event — monitors are blind.
env.storage().instance().set(&DataKey::Admin, &new_admin);
}
}
```

## The fix

Emit an event after every admin-level storage mutation:

```rust
#[contractimpl]
impl Token {
pub fn set_admin(env: Env, new_admin: Address) {
env.storage().instance().set(&DataKey::Admin, &new_admin);
env.events().publish(
(symbol_short!("admin"), symbol_short!("set_adm")),
new_admin,
);
}
}
```

If an event emit is genuinely unnecessary for a specific function, suppress the finding:

```rust
// sanctifier:ignore[SANCT_ADMIN_EVENT_MISSING]
pub fn migrate(env: Env) {
env.storage().instance().set(&DataKey::Version, &2u32);
}
```

## How Sanctifier detects it

The rule uses a `syn::visit::Visit` pass. For every public function in a `#[contractimpl]`
block whose name matches the admin/config heuristic, it runs a body visitor that sets two
flags: `has_mutation` (`.set`/`.update`/`.remove` on a storage receiver) and `has_event`
(`.events()` in any receiver chain, or a call to `publish`/`emit`/`log`). A violation is
emitted only when `has_mutation && !has_event`.

`#[cfg(test)]` modules and functions annotated with
`// sanctifier:ignore[SANCT_ADMIN_EVENT_MISSING]` are skipped.

**Limitations:** detection is name-based, so an admin function with an atypical name is a
false negative. A function that delegates its event emit to an opaque helper may also be
missed. Conversely, a function that genuinely does not need an event (e.g. an internal
migration guard) can be suppressed.

## References

- Soroban docs — [Events](https://soroban.stellar.org/docs/fundamentals-and-concepts/events)
- Related: [`auth_gap`](auth_gap.md), [`state_write_in_view`](state_write_in_view.md)
6 changes: 6 additions & 0 deletions tooling/sanctifier-core/src/finding_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub const UNBOUNDED_STORAGE: &str = "SANCT_UNBOUNDED_STORAGE";
pub const SANCT_VIEW_PANIC: &str = "SANCT_VIEW_PANIC";
pub const ALLOWANCE_RACE: &str = "SANCT_ALLOWANCE_RACE";
pub const STATE_WRITE_IN_VIEW: &str = "SANCT_STATE_WRITE_IN_VIEW";
pub const SANCT_ADMIN_EVENT_MISSING: &str = "SANCT_ADMIN_EVENT_MISSING";
pub const DIVISION_BY_ZERO: &str = "S018";
pub const TIER_BOUNDARY_OFF_BY_ONE: &str = "S022";
pub const MISSING_RESERVE_AUTH: &str = "S023";
Expand Down Expand Up @@ -213,6 +214,11 @@ pub fn all_finding_codes() -> Vec<FindingCode> {
description:
"Getter/view-style function performs a storage write; callers expect it to be read-only",
},
FindingCode {
code: SANCT_ADMIN_EVENT_MISSING,
category: "events",
description: "Admin/config-change function mutates storage without emitting a corresponding on-chain event",
},
FindingCode {
code: DIVISION_BY_ZERO,
category: "arithmetic",
Expand Down
Loading
Loading