Skip to content

fix: backport security fixes from app-version branch#24

Closed
SlavaSereb wants to merge 1 commit into
mainfrom
fix/feedback-review
Closed

fix: backport security fixes from app-version branch#24
SlavaSereb wants to merge 1 commit into
mainfrom
fix/feedback-review

Conversation

@SlavaSereb

@SlavaSereb SlavaSereb commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Backports critical security fixes from the app-version branch to main to ensure both branches have consistent security hardening.

Security Improvements

FireblocksSigner.ts:

  • Add externalTxId parameter for idempotent signing requests (prevents duplicate transactions on retry)
  • Replace fixed 3-second polling with exponential backoff (3s → 30s ceiling)
  • Add 30-minute hard timeout to prevent infinite polling during cold storage approvals
  • Fix stale log message that never updated during polling loop

helpers.ts:

  • concatSignature: Validate recovery ID v ∈ {0, 1} (reject malformed signatures)
  • concatSignature: Validate signature is exactly 128 hex characters
  • concatSignature: Normalize high-S signatures via @noble/secp256k1 for node compatibility
  • validateAddress: Support multisig addresses (SM prefix for mainnet, SN for testnet)
  • untilBurnHeightForCycles: Fix off-by-one error in PoX cycle calculation

Test Plan

  • Verify signing with externalTxId works correctly
  • Verify exponential backoff behavior in polling
  • Verify timeout triggers after 30 minutes
  • Verify multisig addresses (SM/SN) are accepted
  • Verify high-S signatures are normalized
  • Verify invalid v values are rejected
  • Verify cycle calculation returns correct burn height

Related

These fixes address issues identified in the June 2026 security review. See CODE_REVIEW_COMPREHENSIVE_JUNE_2026.md in the app repo for full details.

Security improvements:
- FireblocksSigner: Add externalTxId for idempotency (prevents duplicate signing)
- FireblocksSigner: Replace fixed 3s poll with exponential backoff (3s→30s)
- FireblocksSigner: Add 30-minute timeout to prevent infinite polling
- concatSignature: Validate v ∈ {0,1} (reject invalid recovery IDs)
- concatSignature: Validate signature length (128 hex chars)
- concatSignature: Normalize high-S signatures via @noble/secp256k1
- validateAddress: Support multisig addresses (SM/SN prefixes)
- untilBurnHeightForCycles: Fix off-by-one error in cycle calculation
@SlavaSereb SlavaSereb closed this Jun 9, 2026
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.

1 participant