Skip to content

Commit d7f76b5

Browse files
authored
Cosmos improve (#127)
* cosmos update 1 * cosmos update 2 * cosmos update 3 * rm internal data * renumber * ibc * bump cosmos skill version * proper checklist with progressive disclosure * rm duplicates * workflow instaed of checklist * better workflows * correct version * main skill saves findings
1 parent 17ba9fa commit d7f76b5

File tree

11 files changed

+2508
-692
lines changed

11 files changed

+2508
-692
lines changed

.claude-plugin/marketplace.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@
3131
},
3232
{
3333
"name": "building-secure-contracts",
34-
"version": "1.0.1",
34+
"version": "1.1.0",
3535
"description": "Comprehensive smart contract security toolkit based on Trail of Bits' Building Secure Contracts framework. Includes vulnerability scanners for 6 blockchains and 5 development guideline assistants.",
3636
"author": {
37-
"name": "Omar Inuwa"
37+
"name": "Omar Inuwa && Paweł Płatek"
3838
},
3939
"source": "./plugins/building-secure-contracts"
4040
},

plugins/building-secure-contracts/.claude-plugin/plugin.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
{
22
"name": "building-secure-contracts",
3-
"version": "1.0.1",
3+
"version": "1.1.0",
44
"description": "Comprehensive smart contract security toolkit based on Trail of Bits' Building Secure Contracts framework. Includes vulnerability scanners for 6 blockchains and 5 development guideline assistants.",
55
"author": {
6-
"name": "Omar Inuwa",
6+
"name": "Omar Inuwa && Paweł Płatek",
77
"email": "opensource@trailofbits.com",
88
"url": "https://github.com/trailofbits"
99
}
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
# Cosmos Vulnerability Scanner — Update Log
2+
3+
## Third Update (2026-03-18) — 8 Missing Bug Classes Added
4+
5+
### Summary
6+
Added 8 missing vulnerability classes identified from real-world Cosmos findings catalog. Skill now covers 28 VULNERABILITY_PATTERNS + 16 IBC patterns. No existing patterns were duplicated.
7+
8+
### New Patterns in VULNERABILITY_PATTERNS.md
9+
- **§20 expanded**: ROUNDING ERRORS → ARITHMETIC ERRORS. Added wrapping overflow (CWA-2024-002), wrong operand (Osmosis $5M), negative value validation (Barberry vesting poisoning)
10+
- **§22 expanded**: Added CacheContext event leak pattern (Huckleberry — events persist after state rollback, enables bridge spoofing)
11+
- **§24 expanded**: EVMOS/ETHERMINT-SPECIFIC → EVM/COSMOS STATE DESYNC. Full precompile atomicity section with 6 detection patterns from 7 findings ($7M+ real losses)
12+
- **§25 new**: STORAGE KEY DESIGN FLAWS — string concatenation collisions, prefix iterator malleability, redelegation bypass
13+
- **§26 new**: CONSENSUS VALIDATION GAPS — vote extension forgery (SEDA), block timestamp manipulation (CometBFT Tachyon), vesting on blocked addresses
14+
- **§27 new**: CIRCUIT BREAKER AUTHZ BYPASS — x/circuit scope escalation
15+
- **§28 new**: MERKLE PROOF / CRYPTOGRAPHIC VERIFICATION FLAWS — ICS-23 proof forgery (Dragonberry), IAVL RangeProof (Dragonfruit/$566M), ECDSA malleability (Gravity Bridge)
16+
17+
### New Pattern in IBC_VULNERABILITY_PATTERNS.md
18+
- **§16 new**: IBC REENTRANCY / CEI VIOLATIONS — OnTimeoutPacket reentrancy (ASA-2024-007, $150M+ at risk), Terra IBC hooks ($6.4M stolen)
19+
20+
### SKILL.md Updates
21+
- Updated description, Step 2 checklist, IBC reference count, and Priority Guidelines
22+
23+
---
24+
25+
## Second Update (2026-03-06) — v0.53.x Re-validation + 4 New Patterns
26+
27+
### Summary
28+
Re-validated all 9 existing patterns against cosmos-sdk v0.53.x (commit `82fcb05ceb`, CometBFT v0.38.21, `cosmossdk.io/math` v1.5.3, `cosmossdk.io/collections` v1.3.1). Added 4 new vulnerability patterns discovered during verification. Skill now covers 13 patterns.
29+
30+
### Existing Pattern Corrections
31+
32+
**Pattern 4.4 (Slow ABCI Methods)**:
33+
- Added `HasABCIEndBlock` (returns `[]abci.ValidatorUpdate, error`) as a second EndBlock interface alongside `HasEndBlocker` (returns `error`). Used by x/staking. The skill previously only mentioned one variant.
34+
- Added `HasPrecommit` and `HasPrepareCheckState` as additional lifecycle hooks (confirmed in `core/appmodule/module.go:42-53`). These run after EndBlock and are potential DoS surfaces.
35+
36+
**Pattern 4.5 (ABCI Panics)**:
37+
- Added note about `ValidateBasic()` deprecation (`types/tx_msg.go:102`: "deprecated and now facultative"). This increases the likelihood of invalid inputs reaching panic-prone code in ABCI methods.
38+
39+
### New Patterns Added
40+
41+
**4.10 Unbounded Pagination / Query DoS** (HIGH):
42+
- `types/query/pagination.go` `DefaultLimit=100` only applies when `Limit==0`. Any non-zero client-supplied limit is accepted without cap.
43+
- Recent fix `d9d77304fd` partially addressed this in `x/auth/tx` and `x/bank/keeper/genesis.go`, but core pagination still lacks a hard maximum.
44+
45+
**4.11 Event Override / Suppression** (MEDIUM):
46+
- New `EventManager.OverrideEvents()` method added in commit `2ddb9ac0a9` (`types/events.go:63`). Replaces all previously emitted events.
47+
- If called in module code, can silently suppress events from auth, bank, IBC — breaking indexers and relayers.
48+
49+
**4.12 Unordered Transaction Replay** (HIGH):
50+
- SDK v0.53 supports `TxBody.Unordered=true` with timestamp-based replay protection.
51+
- Configurable via `WithMaxUnorderedTxTimeoutDuration` (default 10min) and `WithUnorderedTxGasCost` (default 2240).
52+
- `x/auth` PreBlocker handles `RemoveExpiredUnorderedNonces` cleanup.
53+
- Risks: insufficient timeout, nonce flooding DoS, ordering assumptions in app logic.
54+
55+
**4.13 Missing msg_server Validation** (HIGH):
56+
- `ValidateBasic()` is now deprecated and facultative (`types/tx_msg.go:102`).
57+
- Modules migrating away from `ValidateBasic` may forget to add equivalent validation in `msg_server.go` handlers.
58+
- Authority/signer checks must be in the handler, not just in the removed `ValidateBasic`.
59+
60+
### Verification Notes
61+
62+
All patterns confirmed against the live codebase:
63+
- `sdk.Msg = proto.Message` (types/tx_msg.go:19) — confirmed
64+
- 0 `GetSigners() []AccAddress` methods in x/ — confirmed (only `LegacyMsg` interface defines it)
65+
- 74 `cosmos.msg.v1.signer` annotations across 22 proto files — confirmed
66+
- `appmodule.HasBeginBlocker`, `HasEndBlocker`, `HasPreBlocker` in core/appmodule/module.go:65-89 — confirmed
67+
- `module.HasABCIEndBlock` in types/module/module.go:237-241 — confirmed (different return type!)
68+
- `appmodule.HasPrecommit`, `HasPrepareCheckState` in core/appmodule/module.go:42-53 — confirmed
69+
- `x/auth/module.go` and `x/upgrade/module.go` implement `PreBlock` — confirmed
70+
- `time.Now()` only in test files under x/, not consensus code — confirmed
71+
- `go func` only in generated `pb.gw.go` files under x/ — confirmed
72+
- `blockedAddrs` mechanism in x/bank/keeper/keeper.go — confirmed
73+
- `QuoTruncate`/`QuoRoundUp` on `math.LegacyDec` in math/legacy_dec.go — confirmed
74+
- No handler.go files for legacy SDK handler pattern under x/ — confirmed
75+
- 22 modules under x/ (no x/accounts in this version) — confirmed
76+
- `collections` framework used extensively (81+ occurrences in keeper.go files) — confirmed
77+
78+
---
79+
80+
# First Update (2026-03-06)
81+
82+
## Context
83+
84+
The skill's vulnerability patterns were written for Cosmos SDK ~v0.46 and had not been updated for the breaking changes in v0.47–v0.53. The current cosmos-sdk codebase (v0.53.x, Go 1.23, `cosmossdk.io/math` v1.5.3) was used as the reference for validation.
85+
86+
---
87+
88+
## 1. Mismatched Pattern Summary in SKILL.md
89+
90+
**Problem**: [SKILL.md:98–109](SKILL.md) Section 5 listed 9 CosmWasm-specific patterns (Missing Denom Validation, Reentrancy via Submessages, etc.) that did not correspond to the actual 9 Cosmos SDK patterns defined in [VULNERABILITY_PATTERNS.md](resources/VULNERABILITY_PATTERNS.md). The summary and the resource file described completely different vulnerability classes.
91+
92+
**Fix**: Replaced the summary to accurately reflect the 9 patterns in the resource file.
93+
94+
---
95+
96+
## 2. Pattern 4.1 — GetSigners: Rewritten for Proto Annotations
97+
98+
**Problem**: The pattern described `GetSigners() []sdk.AccAddress` as a method on message types. This interface was removed in SDK v0.47. In the current codebase:
99+
100+
- `sdk.Msg` is just `proto.Message`[types/tx_msg.go:19](cosmos-sdk/types/tx_msg.go)
101+
- No `func.*GetSigners.*[]sdk.AccAddress` methods exist anywhere in `x/`
102+
- Signers are declared via `cosmos.msg.v1.signer` proto annotations — e.g. [proto/cosmos/gov/v1/tx.proto:60](cosmos-sdk/proto/cosmos/gov/v1/tx.proto)
103+
- Resolution happens in `x/tx/signing`[x/tx/signing/context.go:358](cosmos-sdk/x/tx/signing/context.go) (`func (c *Context) GetSigners`)
104+
105+
**Fix**: Rewrote pattern to describe the modern vulnerability: mismatch between `cosmos.msg.v1.signer` proto annotation and the field actually used for authorization in `msg_server.go`. Added detection patterns for wrong field annotation and proto vs handler mismatch.
106+
107+
---
108+
109+
## 3. Pattern 4.2 — Non-Determinism: Math Type References Updated
110+
111+
**Problem**: Pattern referenced `sdk.Int`, `sdk.Dec`, `sdk.NewDec()` which are removed/deprecated. The codebase uses:
112+
113+
- `math.Int` / `math.NewInt()` from `cosmossdk.io/math` — 186+ occurrences across `x/` modules
114+
- `math.LegacyDec` / `math.LegacyNewDec()` — 83+ occurrences across `x/` modules
115+
- Zero occurrences of `sdk.NewInt()` or `sdk.NewDec()` in `x/` Go source (only in legacy tests via type aliases)
116+
117+
**Fix**: Updated all type references: `sdk.Int``math.Int`, `sdk.Dec``math.LegacyDec`.
118+
119+
---
120+
121+
## 4. Pattern 4.3 — Message Priority: Updated for ABCI 2.0
122+
123+
**Problem**: Pattern only discussed `CheckTx` priority. SDK v0.50 introduced ABCI 2.0 with `PrepareProposal` and `ProcessProposal` (267 occurrences in `baseapp/`), which give the block proposer direct control over transaction ordering.
124+
125+
**Fix**: Added `PrepareProposal`/`ProcessProposal` as the primary mechanism for transaction priority. Updated code examples.
126+
127+
**Evidence**: [baseapp/abci.go](cosmos-sdk/baseapp/abci.go)`FinalizeBlock`, `PrepareProposal`, `ProcessProposal` are the ABCI 2.0 entry points. `DeliverTx` still exists internally but is called within `FinalizeBlock`.
128+
129+
---
130+
131+
## 5. Pattern 4.4 — Slow ABCI Methods: Updated Interfaces
132+
133+
**Problem**: Pattern used standalone `BeginBlocker(ctx sdk.Context, k keeper.Keeper)` function signatures. In SDK v0.50+, modules implement interfaces:
134+
135+
- `appmodule.HasBeginBlocker` — method `BeginBlock(context.Context) error`
136+
- `appmodule.HasEndBlocker` — method `EndBlock(context.Context) error`
137+
- Module manager calls these via [types/module/module.go:776–825](cosmos-sdk/types/module/module.go)
138+
- `PreBlocker` is new (40 occurrences in codebase)
139+
140+
**Evidence**:
141+
```
142+
x/mint/module.go: _ appmodule.HasBeginBlocker = AppModule{}
143+
x/staking/module.go: _ appmodule.HasBeginBlocker = AppModule{}
144+
x/gov/module.go: _ appmodule.HasEndBlocker = AppModule{}
145+
x/feegrant/module.go: _ appmodule.HasEndBlocker = AppModule{}
146+
```
147+
148+
**Fix**: Updated all code examples to use `func (am AppModule) BeginBlock(ctx context.Context) error` and `EndBlock` signatures. Added `PreBlock` as a new attack surface. Updated checklist.
149+
150+
---
151+
152+
## 6. Pattern 4.5 — ABCI Panics: Updated Types and Deprecated Params
153+
154+
**Problem**: Two sub-issues:
155+
156+
1. **Math types**: Pattern used `sdk.NewDec()`, `sdk.NewInt()`. Current SDK uses `math.NewInt()`, `math.LegacyNewDecFromStr()`, and panic-prone `Must*` variants like `math.LegacyMustNewDecFromStr()`.
157+
158+
2. **SetParamSet**: Pattern described `SetParamSet` panics. The entire `x/params` module is deprecated:
159+
- [x/params/module.go:61](cosmos-sdk/x/params/module.go): `"Deprecated: the params module is deprecated and will be removed in the next Cosmos SDK major release."`
160+
- [x/params/keeper/keeper.go:15](cosmos-sdk/x/params/keeper/keeper.go): keeper also deprecated
161+
- Modern modules store params directly in keeper state via `collections`
162+
163+
**Fix**: Updated math type references. Replaced `SetParamSet` panic pattern with direct keeper param storage pattern. Added `Must*` variant warning as a new detection pattern.
164+
165+
---
166+
167+
## 7. Pattern 4.7 — Rounding Errors: Updated Types
168+
169+
**Problem**: All references used `sdk.Dec`, `sdk.NewDec()`, `sdk.ZeroDec()`. These are gone.
170+
171+
**Evidence**: `QuoTruncate` and `QuoRoundUp` still exist on `math.LegacyDec`[math/legacy_dec.go](cosmos-sdk/math/legacy_dec.go) (5 occurrences of QuoTruncate/QuoCeil in codebase). The vulnerability pattern itself is still valid.
172+
173+
**Fix**: Updated all type references to `math.LegacyDec`, `math.LegacyNewDec()`, `math.LegacyZeroDec()`.
174+
175+
---
176+
177+
## 8. Pattern 4.8 — Unregistered Handler: Marked Legacy Only
178+
179+
**Problem**: Pattern described `NewHandler` / `sdk.Handler` switch-case pattern. This is completely gone from the SDK:
180+
181+
- Zero `handler.go` files exist under `x/`
182+
- Zero matches for `NewHandler` or `sdk.Handler` in Go files under `x/`
183+
- All 16 modules use `msg_server.go` with protobuf service registration:
184+
```
185+
x/auth/keeper/msg_server.go
186+
x/bank/keeper/msg_server.go
187+
x/gov/keeper/msg_server.go
188+
x/staking/keeper/msg_server.go
189+
... (16 total)
190+
```
191+
192+
**Fix**: Marked pattern as legacy-only (pre-v0.47). Added modern equivalent: stub/no-op `msg_server.go` implementations where a proto RPC method is technically implemented but does nothing. The Go compiler catches missing methods (unlike the old switch-case), but stub implementations are still a risk.
193+
194+
---
195+
196+
## 9. Scanning Workflow: Updated Throughout
197+
198+
**Changes in [SKILL.md](SKILL.md) Section 6**:
199+
200+
| Old | New |
201+
|-----|-----|
202+
| `handler.go` — Message handlers (legacy) | `keeper/msg_server.go` — Message handlers (protobuf service) |
203+
| `abci.go` — BeginBlocker/EndBlocker | `module.go` — appmodule interface implementations |
204+
| `types/msgs.go`, `GetSigners()` | `proto/.../tx.proto` with `cosmos.msg.v1.signer` annotations |
205+
| `DeliverTx` | `FinalizeBlock` |
206+
| `CheckTx` priority only | `PrepareProposal` / `ProcessProposal` / `CheckTx` |
207+
| `sdk.Dec` / `sdk.Int` | `math.LegacyDec` / `math.Int` |
208+
209+
**Quick Reference Checklist** updated similarly — signer check now references proto annotations, ABCI check includes PreBlock, testing uses `FinalizeBlock`.
210+
211+
---
212+
213+
## Files Modified
214+
215+
- [SKILL.md](SKILL.md) — Full rewrite of sections 3, 5, 6, 7, 9, 10, 11
216+
- [resources/VULNERABILITY_PATTERNS.md](resources/VULNERABILITY_PATTERNS.md) — All 9 patterns updated
217+
218+
## Validation Method
219+
220+
Each pattern was validated against the cosmos-sdk codebase against `cosmos-sdk` (v0.53.x, commit `82fcb05ceb`) using grep/glob searches to confirm:
221+
- Which interfaces/types still exist
222+
- Which have been removed or deprecated
223+
- What replaced them
224+
- Occurrence counts to verify patterns are real

0 commit comments

Comments
 (0)