1be62a4 Fix 14 flaws from adversarial code fuzz, add pending refund queue - makenotwork - Git

Branch or tag

Fix 14 flaws from adversarial code fuzz, add pending refund queue Security and correctness fixes found by systematic code fuzzing: - Scan OOM guard: enforce SCAN_MAX_MEMORY_BYTES before downloading files - Validate key code rejects empty word segments ("----") - Project image confirm validates S3 key prefix - SyncKit auth uses dummy hash to prevent user enumeration - SyncUser extractor checks user suspension - Subscription tier delete wrapped in transaction (TOCTOU fix) - 2FA failed attempts count toward account lockout - CSRF body buffer increased to match global 1MB limit - Import route gets 15MB body limit override - License key revocation LIMIT 1000 removed - User purchases query deduped with DISTINCT ON - YARA scanner gets 30s native timeout Pending refund queue (migration 063): unmatched charge.refunded webhooks stored for later matching instead of silently dropped. Checkout handler checks for pending refunds after completing transactions. Scheduler escalates unmatched refunds >24h old via alert email.

Co-Authored-ByClaude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: Max J. <87768334+MaxJMath@users.noreply.github.com> - 2026-04-25 20:07 UTC

Commit:

1be62a40f7453a6589adda9d02d1836a537b24ea

Parent:

514ead9

22 files changed,

+413 insertions,

-34 deletions

Mserver/docs/todo.md

+58

OldNewLine

@@ -92,6 +92,64 @@

---

## File Scanning — Future Improvements

Files > 100 MB are now held for review instead of downloaded into RAM. Next steps:

### Background scan queue (next)

100

- [ ] Add `scan_queue` table (s3_key, file_type, user_id, status, created_at)

101

- [ ] Enqueue oversized files from `scan_and_classify` instead of blanket HeldForReview

102

- [ ] Scheduler picks up queued scans, streams from S3 to temp file, scans from disk

103

- [ ] Update entity scan status + notify creator on completion

104

- [ ] ClamAV already supports chunked `INSTREAM` — use it for streaming scans

105

- [ ] SHA-256 is naturally streaming — hash in chunks during download

106

- [ ] YARA requires full buffer — memory-map the temp file or skip YARA for large files

107

108

### Separate scanning service (later, when traffic justifies)

109

- [ ] Extract scan worker into standalone binary (same crate, different bin target)

110

- [ ] Worker polls scan_queue, runs on dedicated machine with more RAM

111

- [ ] Allows horizontal scaling independently of request serving

112

- [ ] Consider GPU-accelerated analysis if volume warrants it

113

114

### Other scanning hardening

115

- [ ] Add timeout to YARA scanning (currently unbounded; crafted input could stall)

116

- [ ] Cap ClamAV response buffer size (currently unbounded `read_to_end`)

117

- [ ] Nested archive detection: check magic bytes, not just file extensions

118

119

---

120

121

## Code Fuzz Findings (2026-04-25)

122

123

Bugs found during adversarial code review. Ordered by severity.

124

125

### Critical

126

- [x] ~~20 GB file downloaded into RAM for scanning — `SCAN_MAX_MEMORY_BYTES` was dead code (`routes/storage/mod.rs:91`). Fixed: size guard added to `scan_and_classify`.~~

127

128

### Serious

129

- [x] ~~Refund-before-payment webhook silently lost. Fixed: unmatched refunds stored in `pending_refunds` table (migration 063). Checkout handler checks for pending refunds after completing a transaction. Scheduler escalates unmatched refunds >24h old via admin alert email.~~

130

- [x] ~~`validate_key_code` accepts `"----"` — empty word segments pass `all()` vacuously. Fixed: added `part.is_empty()` check + tests.~~

131

- [x] ~~Project image confirm missing S3 key prefix validation. Fixed: added `starts_with` user ID check in `project_image_confirm`.~~

132

- [x] ~~SyncKit auth lacks dummy hash. Fixed: added `DUMMY_HASH` + `verify_password` timing equalization.~~

133

- [x] ~~SyncUser extractor does not check user suspension. Fixed: added `get_user_by_id` + `is_suspended()` check.~~

134

- [x] ~~`delete_subscription_tier` TOCTOU. Fixed: wrapped in transaction with `FOR UPDATE` on the tier row.~~

135

136

### Minor

137

- [x] ~~2FA verification has no per-user failed-attempt counter. Fixed: reuses `increment_failed_login` — failed 2FA attempts count toward account lockout (5 attempts, 15 min). Reset on success.~~

138

- [x] ~~CSRF body buffer (64KB) < global body limit (1MB). Fixed: increased buffer to 1MB to match global `RequestBodyLimitLayer`.~~

139

- [x] ~~Import endpoint 10MB size limit unreachable due to 1MB global body limit. Fixed: pulled import route into its own group with 15MB `DefaultBodyLimit` override.~~

140

- [ ] Idempotency check not atomic with operation — concurrent requests both execute (`db/idempotency.rs`). Safe only because underlying ops are themselves idempotent.

141

- [ ] `Slug::from_trusted` used on untrusted URL path segments (`custom_domain.rs:164,182` + ~20 page routes). Safe due to sqlx parameterization but a latent footgun.

142

143

### Note

144

- [x] ~~`get_user_purchases` duplicate rows. Fixed: wrapped query in `DISTINCT ON (p.item_id)` subquery.~~

145

- [x] ~~`revoke_keys_by_transaction` LIMIT 1000. Fixed: removed the cap — bulk UPDATE already has no limit, SELECT now matches.~~

146

- [x] ~~YARA scanning has no timeout. Fixed: `scanner.set_timeout(30s)` via yara-x native API.~~

147

- [ ] 7-day SyncKit JWT with no per-user revocation (`constants.rs:37`). Stolen token usable for full window.

148

- [ ] Nested archive detection is extension-based only, not magic bytes (`scanning/archive.rs:81`).

149

- [ ] No rate limiting on read API routes — enables enumeration of tags, categories, domains (`api/mod.rs:366`).

150

151

---

152

153

## Content Fingerprinting — Remaining

154

- [ ] Invisible image watermarks — LSB encoding (stub exists at `fingerprint/watermark_image.rs`)

155

- [ ] Invisible audio watermarks — spread-spectrum (stub exists at `fingerprint/watermark_audio.rs`)

Mserver/src/csrf.rs

-2

OldNewLine

@@ -201,9 +201,11 @@

201

return (StatusCode::FORBIDDEN, "CSRF token required").into_response();

202

}

203

204

// Buffer the body to extract _csrf, then reconstruct the request

204

// Buffer the body to extract _csrf, then reconstruct the request.

205

// Limit matches the global RequestBodyLimitLayer (1 MB) so that any

206

// form body accepted by the server can have its CSRF token extracted.

205

207

let (parts, body) = request.into_parts();

206

let bytes = match axum::body::to_bytes(body, 1024 * 64).await {

208

let bytes = match axum::body::to_bytes(body, 1024 * 1024).await {

207

209

Ok(b) => b,

208

210

Err(_) => {

209

211

return (StatusCode::BAD_REQUEST, "Request body too large").into_response();

Mserver/src/synckit_auth.rs

OldNewLine

@@ -109,6 +109,15 @@

109

return Err(AppError::Unauthorized);

110

}

111

112

// Verify user is not suspended (JWT may outlive suspension)

113

let user = crate::db::users::get_user_by_id(&state.db, claims.sub)

114

.await

115

.map_err(|_| AppError::Internal(anyhow::anyhow!("Failed to verify sync user")))?

116

.ok_or(AppError::Unauthorized)?;

117

if user.is_suspended() {

118

return Err(AppError::Unauthorized);

119

}

120

112

121

Ok(SyncUser {

113

122

user_id: claims.sub,

114

123

app_id: claims.app,

Mserver/src/db/license_keys.rs

-1

OldNewLine

@@ -307,7 +307,7 @@

307

) -> Result<u64> {

308

// Get all key IDs for this transaction

309

let key_ids: Vec<LicenseKeyId> = sqlx::query_scalar(

310

"SELECT id FROM license_keys WHERE transaction_id = $1 AND revoked_at IS NULL LIMIT 1000",

310

"SELECT id FROM license_keys WHERE transaction_id = $1 AND revoked_at IS NULL",

311

)

312

.bind(transaction_id)

313

.fetch_all(&mut *conn)

Mserver/src/db/mod.rs

OldNewLine

@@ -59,6 +59,7 @@

pub(crate) mod tips;

pub(crate) mod project_members;

pub(crate) mod idempotency;

pub(crate) mod pending_refunds;

pub(crate) mod webhook_events;

pub use id_types::*;

Mserver/src/db/subscriptions.rs

+16

-3

OldNewLine

@@ -137,27 +137,40 @@

137

138

/// Delete a subscription tier. Soft-deletes (sets is_active=false) if any

139

/// subscriptions reference it; hard-deletes otherwise.

140

///

141

/// Uses a transaction with FOR UPDATE to prevent a TOCTOU race where a

142

/// subscription could be created between the existence check and the delete.

140

143

#[tracing::instrument(skip_all)]

141

144

pub async fn delete_subscription_tier(pool: &PgPool, id: SubscriptionTierId) -> Result<()> {

145

let mut tx = pool.begin().await?;

146

147

// Lock the tier row to serialize against concurrent subscription creation

148

sqlx::query("SELECT id FROM subscription_tiers WHERE id = $1 FOR UPDATE")

149

.bind(id)

150

.fetch_optional(&mut *tx)

151

.await?

152

.ok_or(sqlx::Error::RowNotFound)?;

153

142

154

let has_subscriptions: bool = sqlx::query_scalar(

143

155

"SELECT EXISTS(SELECT 1 FROM subscriptions WHERE tier_id = $1)",

144

156

)

145

157

.bind(id)

146

.fetch_one(pool)

158

.fetch_one(&mut *tx)

147

159

.await?;

148

160

149

161

if has_subscriptions {

150

162

sqlx::query("UPDATE subscription_tiers SET is_active = false WHERE id = $1")

151

163

.bind(id)

152

.execute(pool)

164

.execute(&mut *tx)

153

165

.await?;

154

166

} else {

155

167

sqlx::query("DELETE FROM subscription_tiers WHERE id = $1")

156

168

.bind(id)

157

.execute(pool)

169

.execute(&mut *tx)

158

170

.await?;

159

171

}

160

172

173

tx.commit().await?;

161

174

Ok(())

162

175

}

163

176

Mserver/src/db/transactions.rs

+18

-15

OldNewLine

@@ -312,21 +312,24 @@

312

pub async fn get_user_purchases(pool: &PgPool, user_id: UserId) -> Result<Vec<DbPurchaseRow>> {

313

let purchases = sqlx::query_as::<_, DbPurchaseRow>(

314

r#"

315

SELECT

316

p.item_id,

317

i.title,

318

u.username as creator,

319

i.item_type,

320

p.purchased_at,

321

(i.price_cents = 0) as is_free,

322

lk.key_code as license_key_code

323

FROM purchases p

324

JOIN items i ON p.item_id = i.id

325

JOIN projects proj ON i.project_id = proj.id

326

JOIN users u ON proj.user_id = u.id

327

LEFT JOIN license_keys lk ON lk.item_id = p.item_id AND lk.owner_id = p.buyer_id AND lk.revoked_at IS NULL

328

WHERE p.buyer_id = $1

329

ORDER BY p.purchased_at DESC

315

SELECT * FROM (

316

SELECT DISTINCT ON (p.item_id)

317

p.item_id,

318

i.title,

319

u.username as creator,

320

i.item_type,

321

p.purchased_at,

322

(i.price_cents = 0) as is_free,

323

lk.key_code as license_key_code

324

FROM purchases p

325

JOIN items i ON p.item_id = i.id

326

JOIN projects proj ON i.project_id = proj.id

327

JOIN users u ON proj.user_id = u.id

328

LEFT JOIN license_keys lk ON lk.item_id = p.item_id AND lk.owner_id = p.buyer_id AND lk.revoked_at IS NULL

329

WHERE p.buyer_id = $1

330

ORDER BY p.item_id, p.purchased_at DESC

331

) deduped

332

ORDER BY purchased_at DESC

330

333

LIMIT 20

331

334

"#,

332

335

)

Mserver/src/scanning/yara.rs

Mserver/src/validation/items.rs

-1

Mserver/src/routes/api/mod.rs

+11

-2

Mserver/src/routes/storage/images.rs

+10

-2

Mserver/src/routes/storage/media.rs

-1

Mserver/src/routes/storage/mod.rs

+23

Mserver/src/routes/storage/uploads.rs

-1

Mserver/src/routes/storage/versions.rs

-1

Mserver/src/routes/synckit/auth.rs

+15

-3

Mserver/src/routes/api/internal/uploads.rs

-1

Mserver/src/routes/pages/public/two_factor.rs

+34

Mserver/src/routes/stripe/webhook/billing.rs

+27

-1

Mserver/src/routes/stripe/webhook/checkout.rs

+46

Aserver/migrations/063_pending_refunds.sql

+18

Aserver/src/db/pending_refunds.rs

+110