Skip to content

fix(security): Fase 4-C low-batch — viable pool + admin cooldown + identity error log (SEV-025 + SEV-027 + SEV-028)#339

Merged
alrimarleskovar merged 2 commits into
mainfrom
fix/fase4-c-sev-025-028-low-batch
May 15, 2026
Merged

fix(security): Fase 4-C low-batch — viable pool + admin cooldown + identity error log (SEV-025 + SEV-027 + SEV-028)#339
alrimarleskovar merged 2 commits into
mainfrom
fix/fase4-c-sev-025-028-low-batch

Conversation

@alrimarleskovar
Copy link
Copy Markdown
Owner

Adevar Labs re-audit — Fase 4 PR-C (final). 3 of 4 Low-severity new findings. SEV-026 (cascade duplication) deferred to Fase 5 as maintenance refactor.

SEV-025 — default constants form an inviable pool

Old: 24 × 416 × 0.74 = 7388 USDC pool float < 10_000 credit → cycle-0 always failed Seed Draw guard.

Fix: DEFAULT_INSTALLMENT_AMOUNT 416 → 600 USDC. Now 24 × 600 × 0.74 = 10_656 USDC > 10_000 credit ✓. Added an invariant assertion to constants test (pool_float >= credit). SDK updated in lockstep for parity test.

SEV-027 — admin can pump score arbitrarily via SCHEMA_PAYMENT

Pool-PDA path is rate-limited by cycle structure; admin-direct PAYMENT had no cooldown.

Fix:

  • New ReputationProfile.last_admin_attest_at: i64 (consumes 8 of 15 _padding bytes, LEN unchanged)
  • New MIN_ADMIN_ATTEST_COOLDOWN_SECS = 60
  • attest.rs: when is_admin && schema == SCHEMA_PAYMENT, require 60s elapsed
  • Field bumped only on admin-issued attests (pool-PDA flow unaffected)

SEV-028 — refresh_identity engole erros

Old: Err(_) arm discarded the validation error. Operators couldn't distinguish actual revocation from config drift.

Fix: capture Err(e) and msg! it before flipping to Revoked. Behavior unchanged (Revoked is still conservative); observability gained.

SEV-026 — DEFERRED

settle_default duplicates cascade math instead of calling crates::math::seize_for_default. Drift risk, not a security issue. Tracked as Fase 5 maintenance refactor.

Validation

cargo check -p roundfi-core              # green
cargo check -p roundfi-reputation        # green
cargo test -p roundfi-core --lib         # 40 passed (incl new viability check)
cargo test -p roundfi-reputation --lib   # 10 passed
pnpm typecheck + lint                    # green
pnpm test:parity                         # 7 passed

Fase 4 status (final)

PR SEVs Severity Status
#337 F4-A: 021 + 022 High × 2 ✅ Open
#338 F4-B: 023 + 024 Medium × 2 ✅ Open
This PR F4-C: 025 + 027 + 028 Low × 3 ✅ Open
(deferred) F4-D: 026 cascade duplication Low (maintenance) Fase 5
(deferred) F4-E: SEV-012 bankrun-in-CI Medium Blocked on #319

After Fase 4 lands, 27 of 28 audit findings closed (only SEV-026 maintenance refactor + SEV-012 upstream-blocked remain).


Generated by Claude Code

…ent cooldown + identity error log (SEV-025 + SEV-027 + SEV-028)

Adevar Labs re-audit Fase 4 PR-C. Three Low-severity new findings.
SEV-026 (settle_default cascade duplication) deferred — drift risk,
not a security issue, refactor can ship separately as Fase 5
maintenance.

SEV-025 Low — default constants form an inviable pool
======================================================

Old defaults: 24 members × 416 USDC installment × 0.74 (after 1%
solidarity + 25% escrow) = 7388 USDC pool float per cycle. Less
than the 10_000 USDC default credit. Cycle 0 claim_payout always
failed with WaterfallUnderflow because the Seed Draw guard requires
the pool to retain 91.6% of credit at first payout.

Fix: bump DEFAULT_INSTALLMENT_AMOUNT 416 → 600 USDC. Now:
  pool_float = 24 × 600 × 0.74 = 10_656 USDC > 10_000 credit ✓

Whitepaper credit + members + cycles unchanged; installment shifted
to make the math close. Added a viability assertion to the constants
test: `pool_float >= credit` is now invariant-guarded against drift.

SDK (sdk/src/constants.ts) updated in lockstep so the Rust↔TS parity
test stays green.

SEV-027 Low — admin can pump score arbitrarily via SCHEMA_PAYMENT
==================================================================

Pool-PDA-issued PAYMENT is rate-limited by the cycle structure
(one per member per cycle). Admin-direct PAYMENT had no cooldown —
admin could pump score arbitrarily by issuing PAYMENT in a tight
loop. Cooldown only existed on SCHEMA_CYCLE_COMPLETE.

Fix:
  - New ReputationProfile field: `last_admin_attest_at: i64`
    (consumes 8 of the 15 _padding bytes; LEN unchanged on disk).
  - New constant MIN_ADMIN_ATTEST_COOLDOWN_SECS = 60.
  - attest.rs handler: when `is_admin && schema == SCHEMA_PAYMENT`,
    require `now - profile.last_admin_attest_at >= 60s`. Conservative
    floor — defeats trivial-loop pumping without blocking legitimate
    manual corrections.
  - Updated last_admin_attest_at on every admin-issued attest. Pool-
    PDA attests do NOT bump this field, so the cooldown only fires
    on admin spam.

SEV-028 Low — refresh_identity engole erros silenciosamente
============================================================

The Err(_) arm in refresh_identity discarded the underlying error
when validate_passport_attestation failed and flipped the record to
Revoked. Operators / monitoring couldn't distinguish "user was
actually revoked" from "bridge service had a config drift" (network
pubkey mismatch, layout shift, owner spoof attempt) — both ended up
as silent Revoked.

Fix: capture the error and emit a msg! before flipping. Status
behavior unchanged (Revoked is still the conservative outcome); the
diff is observability so off-chain monitors can alert on structural
failures vs genuine revocations.

Validation
==========

  $ cargo check -p roundfi-core              # green
  $ cargo check -p roundfi-reputation        # green
  $ cargo test -p roundfi-core --lib         # 40 passed (incl new viability check)
  $ cargo test -p roundfi-reputation --lib   # 10 passed
  $ pnpm typecheck + lint                    # green
  $ pnpm test:parity                         # 7 passed (SDK 600 USDC bump in sync)

Closes Fase 4 PR-C of the Adevar Labs re-audit remediation plan.
SEV-026 (settle_default cascade duplication) deferred to Fase 5 as
non-security maintenance refactor.

https://claude.ai/code/session_01YapZy1Z5gzbV5EammBkSQm
@vercel
Copy link
Copy Markdown

vercel Bot commented May 15, 2026

Deployment failed with the following error:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/alrimarleskovars-projects?upgradeToPro=build-rate-limit

@alrimarleskovar alrimarleskovar merged commit 50db364 into main May 15, 2026
5 of 6 checks passed
alrimarleskovar pushed a commit that referenced this pull request May 15, 2026
…ings

Re-audited against main HEAD 26060c5 after the team merged 18 fix PRs
(#326-#339). 27/28 previous findings confirmed FIXED with verification
diff-by-diff. Quality of fixes is generally excellent (esp. SEV-022
selective pause and SEV-007 demote-on-default).

But 5 new findings surfaced from fresh reading of the corrected code:

⚠ SEV-029 (HIGH — REGRESSION): SEV-016 release_escrow partial-pay
fix introduces deterministic OVERPAY. When partial path triggers
(vault shortfall after settle_default), checkpoint stays at
last_released_checkpoint. Next call recomputes delta_target from the
SAME unchanged last_checkpoint, returning the FULL delta again — not
the remaining. Member can claim partial 100 + full 208 = 308 USDC
when entitled to 208. Overpay drains shared escrow_vault containing
other members' contributions. Exploitable by any member.

SEV-030 (Low): SEV-027 admin attest cooldown only covers
SCHEMA_PAYMENT, leaves SCHEMA_LATE/SCHEMA_DEFAULT uncapped. Admin
compromise can grief score-destruction unboundedly.

SEV-031 (Low): SEV-025 fix updated DEFAULT_INSTALLMENT_AMOUNT but
no runtime solvency check in create_pool. Custom pools with bad
params still create OK and lock funds permanently.

SEV-032 (Info): ReputationConfig padding zeroed by SEV-021
extension. Future schema bumps require account migration.

SEV-033 (Info): Webhook auth fail-open when HELIUS_WEBHOOK_SECRET
unset (only a startup warning) — production deploy footgun.

Score: 7.5 → 7.0/10 (Security 7 → 5.5; Tests 6.5 → 5.5 due to
fixes shipped without negative-path test coverage).

Recommendation: NÃO REMOVER canary cap until SEV-029 fixed and
property test on release_escrow conservation lands.

Process recommendation: every Critical/High fix should ship with
a negative-path bankrun test before merge.

https://claude.ai/code/session_01CiaV9hd9oFqqr7m9ANgfit
@alrimarleskovar alrimarleskovar deleted the fix/fase4-c-sev-025-028-low-batch branch May 17, 2026 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants