Skip to content

[HLR] A1 — Security hotfixes for bsv-sdk 0.8.2 + bsv-wallet 0.3.4 (post-compliance-review) #305

Description

@sgbett

Problem

The 2026-04-08 cross-SDK compliance review identified three high-severity security or silent-corruption findings that must ship as backportable patch releases ahead of any other correctness work. Implementation Strategist noted explicitly: "Never bundle security fixes with architectural churn — they need to be backportable."

This HLR captures cluster A1 (Security hotfixes, P0) from the review's sequencing recommendation.

Target releases

This HLR requires paired patch releases of both gems:

  • bsv-sdk 0.8.1 → 0.8.2 — F1.3 (VarInt) + F5.13 (ARC broadcaster)
  • bsv-wallet 0.3.3 → 0.3.4 — F8.15 (acquire_certificate signature verification — the credential-forgery fix)

The pairing is required because:

  1. The F8.15 fix is in lib/bsv/wallet_interface/, which belongs to bsv-wallet (per bsv-wallet.gemspec file globs)
  2. The F1.3 and F5.13 fixes are in lib/bsv/transaction/ and lib/bsv/network/, which belong to bsv-sdk
  3. A user installing only bsv-wallet 0.3.4 without the bsv-sdk bump would silently miss the F1.3 and F5.13 security fixes

The bsv-wallet.gemspec bsv-sdk dependency must also be tightened from the current stale ~> 0.4 to >= 0.8.2, < 1.0 as part of this PR to enforce the pairing.

Findings in scope

F8.15 — acquire_certificate 'direct' path writes unverified certifier signatures (bsv-wallet)

  • Severity: HIGH (security)
  • Gem: bsv-wallet
  • Location: lib/bsv/wallet_interface/wallet_client.rb:805-816
  • Description: acquire_via_direct constructs a certificate hash from user-supplied fields (type, subject, serial_number, certifier, revocation_outpoint, signature, fields, keyring) and writes it to storage without verifying the certifier's signature. A caller can pass an arbitrary signature: value and it will be persisted as authentic. list_certificates and prove_certificate later treat the record as valid.
  • Why this matters: BRC-52 requires certificates to be verified against the certifier's public key over a canonical hashing of (type, subject, serialNumber, revocationOutpoint, fields). TS's Certificate.verify() implements this check; Ruby omits it entirely. This is a credential forgery primitive — not just a spec divergence.
  • Project team vote: 8 AGREE, 0 DISAGREE. Cryptography Specialist: "A signature-verification bypass masquerading as an API finding. Treat as P0 security." Security Specialist: "The most urgent fix in this review. This should block the next release."
  • Recommended action: Before persisting in acquire_via_direct, run BRC-52 certificate signature verification against the claimed certifier's public key. Reject certificates whose signature doesn't verify with a clear InvalidCertificateError. Never persist unverified data.

F1.3 — VarInt.encode accepts negative integers and silently mis-encodes them (bsv-sdk)

  • Severity: HIGH (silent protocol corruption)
  • Gem: bsv-sdk
  • Location: lib/bsv/transaction/var_int.rb:17-27
  • Description: VarInt.encode(-1) falls into the value < 0xFD branch and calls [value].pack('C'). Ruby's pack('C') silently masks to one byte, so -1 & 0xFF = 0xFF — which is the VarInt marker byte for a 9-byte encoding, not the value -1. A downstream reader will then try to consume 8 more bytes and drift into garbage. There is no guard, and the test suite has no negative-input case.
  • Why this matters: Satoshi CompactSize is an unsigned encoding; negative values are out of domain. The docstring says "non-negative integer" but the implementation does not enforce it. Silent protocol-corrupting output is the worst class of bug — no exception, wrong bytes on the wire.
  • Project team vote: 9 AGREE, 0 DISAGREE. Pragmatic Enforcer: "Concrete silent protocol corruption. One-line guard. Must fix."
  • Recommended action: Add raise ArgumentError, 'varint requires non-negative integer' if value.negative? at the top of VarInt.encode. Also add an explicit upper bound: raise ArgumentError, 'varint value exceeds uint64' if value > 0xFFFF_FFFF_FFFF_FFFF (currently the pack('Q<') raises a cryptic RangeError at 2^64). Add negative-input regression tests.

F5.13 — ARC broadcaster missing failure statuses, wrong content type, missing required headers (bsv-sdk)

  • Severity: HIGH (silent failure → success)
  • Gem: bsv-sdk
  • Location: lib/bsv/network/arc.rb:35-45, 74-100
  • Description: The ARC broadcaster diverges from the TS reference in several ways, the most dangerous of which is missing failure statuses: Ruby's failure detection only recognises REJECTED and DOUBLE_SPEND_ATTEMPTED. TS additionally recognises INVALID, MALFORMED, MINED_IN_STALE_BLOCK, and any ORPHAN-containing extraInfo/txStatus. Ruby therefore treats INVALID, MALFORMED, MINED_IN_STALE_BLOCK, and orphan responses as broadcast successes.
  • Additional divergences (also to fix in this HLR):
    • Content-Type: Ruby sends application/octet-stream; TS sends application/json with an EF-hex body
    • Missing headers: XDeployment-ID, X-CallbackUrl, X-CallbackToken
  • Why this matters: Applications that rely on broadcaster status to gate actions (accepting a payment, marking a token as minted, etc.) will be tricked into trusting un-broadcast or invalid transactions. This is an integrity bug with security consequences.
  • Project team vote: 8 AGREE, 0 DISAGREE. Security Specialist: "Silently treating failure statuses as success is a serious integrity concern." Domain Expert: "ARC missing failure-status set means broadcasts that are actually INVALID/MALFORMED are treated as success; domain-critical."
  • Recommended action:
    • Add the missing failure statuses to the failure-detection predicate: INVALID, MALFORMED, MINED_IN_STALE_BLOCK, and any response containing ORPHAN in extraInfo or txStatus
    • Switch Content-Type to application/json with the body shape { \"rawTx\": tx.to_hex } (EF form when source transactions are available, falling back to plain raw tx)
    • Add XDeployment-ID (random or configured), X-CallbackUrl, X-CallbackToken header support (optional values)

Acceptance criteria

  • F8.15: acquire_certificate 'direct' path verifies certifier signature before persisting; invalid certificates raise InvalidCertificateError (or similar) and are not written to storage. Regression test with a tampered signature. (bsv-wallet)
  • F1.3: VarInt.encode raises ArgumentError on negative or >2^64-1 input. Tests cover -1, -0xFFFFFFFFFFFFFFFF, 2**64, and the boundary 2**64 - 1. (bsv-sdk)
  • F5.13: ARC broadcaster recognises the full TS failure status set; Content-Type set to application/json; optional deployment/callback headers supported. Test with mocked ARC responses for each failure status. (bsv-sdk)
  • bsv-wallet.gemspec bsv-sdk dependency tightened from ~> 0.4 to >= 0.8.2, < 1.0. (bsv-wallet gemspec)
  • All changes ship as bsv-sdk 0.8.2 + bsv-wallet 0.3.4 paired patch release, backportable, no architectural changes bundled.
  • Updated CHANGELOG entries under sdk-0.8.2 and wallet-0.3.4.

Out of scope

  • Any parser/interpreter/BEEF correctness fixes (those belong in 0.9.0 under separate HLRs per the 0.9.0 rollout plan)
  • ECIES, ECDSA, WIF, or other crypto-hardening changes (cluster A4, 0.9.0)
  • Any refactoring, renaming, or new API surfaces
  • Extended Format broadcaster support beyond the simple JSON-with-hex body
  • Retry logic, rate limiting, or other broadcaster feature additions
  • F8.16 issuance-path BRC-104 AuthFetch transport (deferred to Tier B; the signature-verification aspect of F8.16 was already closed as a side effect of F8.15 — see follow-up comment)

Sequencing

This HLR is the first of the compliance-review follow-up HLRs. It is a paired patch release (bsv-sdk 0.8.2 + bsv-wallet 0.3.4) and must land before the 0.9.0 rollout begins. The remaining 6 clusters (A2 through A7 plus C1 conformance suite) are tracked under the 0.9.0 rollout plan.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingproject:hlrHigh-Level Requirement

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions