Skip to content

Lockbox uses assert() on secp256k1 failures; release builds compile the checks out #21

Description

@Rob1Ham

Component: lockbox/src/enclave.cpp
Severity: Low (robustness/availability)

Summary

enclave.cpp checks secp256k1 return values with assert(return_val) — 19 occurrences (e.g., after secp256k1_blinded_musig_partial_sign_without_keyaggcoeff ~L183, keypair sec/pub extraction, tweak ops, nonce parse).

Problems:

  1. Debug builds: a failed secp256k1 call aborts the process — an uncontrolled crash instead of an error response, i.e. a lockbox-wide DoS on any signing-library failure.
  2. Release builds: CMakePresets.json sets CMAKE_BUILD_TYPE=Release, which defines NDEBUG and compiles the asserts out entirely. Execution then continues past a failed call with uninitialized/garbage state (e.g., partial_sig from a failed sign would be memcpy'd into the response).

Suggested direction

Replace assert(return_val) with explicit error propagation (throw / error return to the Crow route), matching the style already used for decrypt_data failures in the same file. At minimum, use a release-safe CHECK macro that always aborts with a logged error.

Found during security review of feature/bip448-web-wallet-mutinynet @ 64d2423.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions