Skip to content

chore: crypto primitives external audit response 1#22272

Open
johnathan79717 wants to merge 1 commit intomerge-train/barretenbergfrom
jh/crypto-primitives-external-audit-response-1
Open

chore: crypto primitives external audit response 1#22272
johnathan79717 wants to merge 1 commit intomerge-train/barretenbergfrom
jh/crypto-primitives-external-audit-response-1

Conversation

@johnathan79717
Copy link
Copy Markdown
Contributor

@johnathan79717 johnathan79717 commented Apr 2, 2026

Audit Context

Addresses finding #2 from the "Aztec - Cryptographic Primitives" external audit: WASM Process DOS via Oversized Polynomial in Prover Commit Path.

In WASM builds, BB_NO_EXCEPTIONS is defined. The C++ throw_or_abort_impl definition calls abort_with_message() which terminates the WASM process on any error. Meanwhile, the header (throw_or_abort_impl.hpp) declares the same function as a WASM_IMPORT from JavaScript, where it throws a catchable JS Error. But the local C++ definition takes priority over the import, so the JS throw is never used.

Changes Made

  • throw_or_abort_impl.cpp: Guard the entire C++ definition with #ifndef __wasm__. In WASM builds, the JS-provided import (which throws a catchable JS Error) is used instead. The header already declares it as WASM_IMPORT("throw_or_abort_impl") with the comment "For a WASM build, this is provided by the JavaScript environment."
  • c_bind.cpp: Update comment documenting the error handling strategy for WASM vs native.

Before: Any throw_or_abort in WASM calls abort_with_message() -> std::abort() -> WASM process dies, PXE crashes.

After: throw_or_abort in WASM calls the JS import which throws new Error(msg). The JS caller catches this as a normal exception. The WASM instance stays alive for subsequent requests.

Checklist

  • Verified native build passes (ninja clean build)
  • Verified WASM build passes (ninja bbapi_objects env_objects in build-wasm-threads)
  • Ran bbapi_tests --gtest_filter=CBind.* (2 passed)

@johnathan79717 johnathan79717 added the ci-barretenberg Run all barretenberg/cpp checks. label Apr 2, 2026
@johnathan79717 johnathan79717 force-pushed the jh/crypto-primitives-external-audit-response-1 branch from ddfb651 to 1d0754e Compare April 2, 2026 14:18
@johnathan79717 johnathan79717 requested a review from ludamad April 2, 2026 14:19
@johnathan79717 johnathan79717 added ci-full Run all master checks. and removed ci-barretenberg Run all barretenberg/cpp checks. labels Apr 2, 2026
@johnathan79717 johnathan79717 marked this pull request as draft April 2, 2026 14:53
@johnathan79717 johnathan79717 removed the request for review from ludamad April 2, 2026 14:53
@johnathan79717 johnathan79717 marked this pull request as ready for review April 2, 2026 15:43
In WASM builds, BB_NO_EXCEPTIONS is defined which routes throw_or_abort
to std::abort(), killing the WASM process on any error with no recovery.

The fix: don't compile the C++ definition of throw_or_abort_impl for
WASM builds (#ifndef __wasm__). The header already declares it as a
WASM_IMPORT from the JS environment, where it throws a catchable JS
Error. With the competing C++ definition removed, the JS import is
used, and errors propagate as JS exceptions back to the TS caller
instead of aborting the process.
@johnathan79717 johnathan79717 force-pushed the jh/crypto-primitives-external-audit-response-1 branch from ea7899c to 4de984d Compare April 2, 2026 16:49
@johnathan79717 johnathan79717 enabled auto-merge (squash) April 2, 2026 17:00
@johnathan79717 johnathan79717 disabled auto-merge April 2, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-full Run all master checks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants