You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: SECURITY.md
+203Lines changed: 203 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -175,3 +175,206 @@ Security alerts from CodeQL are surfaced in the **Security** tab of the reposito
175
175
- Consider the security impact of any medium or low severity issues
176
176
177
177
The workflow uses the `security-extended` and `security-and-quality` query suites to provide comprehensive coverage of potential vulnerabilities.
178
+
179
+
---
180
+
181
+
## Self-Audit Checklist (Smart Contract / DeFi)
182
+
183
+
Use this checklist when reviewing the Soroban contract (`contracts/src/lib.rs`) or proposing changes that affect on-chain logic. Each item must be answered before merging.
184
+
185
+
### Reentrancy
186
+
187
+
| # | Check | Severity | Mitigation | Status |
188
+
|---|-------|----------|------------|--------|
189
+
| 1 | External token transfers occur **after** all state updates (checks-effects-interactions pattern) |**High**| Move `TokenClient::transfer()` calls after storage writes; or add a reentrancy guard if transfers must precede state changes | ☐ Open |
190
+
| 2 | Cross-contract calls do not allow the callee to re-enter and mutate contract state before the first invocation completes |**High**| Verify that Soroban's host-level call-depth limits are sufficient; add a `REENTRANCY_GUARD` flag stored in temporary storage if untrusted contracts are called | ☐ Open |
191
+
| 3 |`claim()` and `refund()` loops call external token contracts on each iteration without intermediate state snapshots |**Medium**| Consider batching transfers or taking a storage snapshot before the loop so a partial failure does not leave state inconsistent | ☐ Open |
192
+
193
+
### Access Control
194
+
195
+
| # | Check | Severity | Mitigation | Status |
196
+
|---|-------|----------|------------|--------|
197
+
| 4 | Admin address is immutable after `initialize()`|**Critical**| Already enforced — `DataKey::Admin` is set once and never updated; verify no `set_admin()` function is added in future PRs | ☐ Pass |
198
+
| 5 | Creator-only functions (`cancel_campaign`, `claim`, `update_metadata`) verify caller matches `campaign.creator`|**High**| Already enforced with `require_auth()` + identity comparison; ensure any new creator-gated function follows the same pattern | ☐ Pass |
199
+
| 6 | No function allows arbitrary address to withdraw funds from any campaign |**Critical**|`claim()` checks `campaign.creator`; `refund()` checks `contributor.require_auth()` and `HasContributed` key; verify no new withdrawal paths bypass these checks | ☐ Open |
200
+
| 7 | Pause mechanism excludes read-only functions to avoid denial-of-service on data reads |**Low**| Already enforced — `require_not_paused()` is called only in state-mutating entry points; verify any new `fn` with side effects does the same | ☐ Pass |
201
+
202
+
### Integer Overflow / Arithmetic
203
+
204
+
| # | Check | Severity | Mitigation | Status |
205
+
|---|-------|----------|------------|--------|
206
+
| 8 |`overflow-checks = true` is set in `Cargo.toml` release profile |**Critical**| Already present on line 27 of `contracts/Cargo.toml`; verify it is never removed or commented out | ☐ Pass |
207
+
| 9 |`pledged_amount + amount <= target_amount` guard prevents both overflow and over-funding |**High**| Already implemented at line 361 of `lib.rs`; verify similar guards exist for any new arithmetic in the contract | ☐ Open |
208
+
| 10 | Token amounts use `i128` (not `u64` or `u128`) to match Soroban token interface |**Medium**| Already using `i128` throughout; verify new fields also use `i128` and never cast without bounds checks | ☐ Pass |
209
+
| 11 | Multiplication before division (or vice versa) does not cause precision loss or overflow |**Medium**| Review any percentage or ratio calculations added in future; prefer `checked_mul().unwrap_or(i128::MAX)` for multiplications that could overflow | ☐ Open |
210
+
| 12 |`contributor_count += 1` (and similar counters) cannot overflow |**Low**| With `overflow-checks = true`, a panic would occur; consider using `saturating_add` if graceful handling is preferred over panic | ☐ Open |
211
+
212
+
### Flash Loan Attack Vectors
213
+
214
+
| # | Check | Severity | Mitigation | Status |
215
+
|---|-------|----------|------------|--------|
216
+
| 13 | Contract does not expose a "donate" or "deposit" function that manipulates internal price/balance snapshots used by other operations |**High**| Verify no function accepts tokens without recording a corresponding contribution or refund; flash loans rely on the ability to manipulate oracle-like state | ☐ Open |
217
+
| 14 | Campaign balance used for eligibility checks reflects actual token balance of the contract, not a stored snapshot |**Medium**| The contract tracks per-campaign token balances via `CampaignTokenBalance`; consider cross-referencing with `TokenClient::balance()` to prevent balance inflation attacks | ☐ Open |
218
+
| 15 | No governance or quorum function relies on a contributor's token balance that could be borrowed for a single transaction |**Medium**|`contributor_count` counts unique addresses (not balances), so flash-loaned tokens cannot inflate voting power; verify the same for any future governance features | ☐ Pass |
219
+
220
+
### Front-Running
221
+
222
+
| # | Check | Severity | Mitigation | Status |
223
+
|---|-------|----------|------------|--------|
224
+
| 16 |`create_campaign` parameters cannot be front-run to replace a legitimate campaign creation with a lookalike |**Medium**|`campaign_id` is a monotonically increasing `u32`, so an attacker can only create their own campaign with a predictable ID; no user-supplied ID exists | ☐ Pass |
225
+
| 17 |`claim()` cannot be front-run by an attacker to redirect funds |**High**|`claim()` transfers only to `campaign.creator` (verified via `require_auth()`); funds always go to the intended recipient regardless of transaction ordering | ☐ Pass |
226
+
| 18 | Deadline-dependent logic (`claim` vs `refund`) is not susceptible to validator timestamp manipulation |**Medium**| Soroban ledger timestamps are bounded by validator consensus; still, avoid tight time windows where a one-slot difference changes fund disposition | ☐ Open |
227
+
| 19 |`contribute()` race condition: two contributions arriving in the same block cannot jointly exceed `target_amount`|**Low**| Each contribution checks `pledged_amount + amount <= target_amount` independently; in practice the gap is bounded by one contribution. Acceptable for MVP | ☐ Open |
228
+
229
+
### Findings Register (Open Items)
230
+
231
+
| ID | Category | Finding | Severity | Mitigation | Assigned To | Due Date |
| F-01 | Reentrancy | Token transfer precedes state update in `contribute()` — external call before storage write violates checks-effects-interactions |**High**| Swap the order: update `pledged_amount`, `contributor_count`, and per-contributor balance **before** calling `TokenClient::transfer()`| — | — |
234
+
| F-02 | Access Control | No two-step admin transfer mechanism exists |**Low**| If admin rotation is needed, implement a two-step pattern (propose + accept) to prevent accidental lockout | — | — |
235
+
| F-03 | Arithmetic | Counter increments use raw `+=` rather than `checked_add`|**Low**| With `overflow-checks=true` these will panic on overflow, which is acceptable; upgrade to `checked_add()` for explicit error handling if desired | — | — |
236
+
237
+
---
238
+
239
+
## External Audit Firm Template
240
+
241
+
Use this template when engaging an external security firm to audit the Soroban smart contract. Fill in the fields marked `[...]` before sending.
0 commit comments