security: remove crypto-browser.js (broken Schnorr leaks private key)#10
Merged
melvincarvalho merged 1 commit intoMay 12, 2026
Conversation
…rial crypto-browser.js contained a completely broken "Schnorr" implementation that: - Derived public keys via SHA-256(private_key) instead of secp256k1 scalar multiplication - Created "signatures" as SHA-256(private_key || event_id) duplicated to 128 hex chars - This means signatures contained material directly derived from the private key, enabling private key recovery through known-plaintext analysis While the file is not imported in the current bundled build (background.js imports from crypto.js which uses @noble/secp256k1), its presence in the source tree is an active hazard: any future refactor, build configuration change, or browser field mapping in package.json could silently activate this broken implementation. The correct implementation lives in src/crypto.js using @noble/secp256k1 with proper Schnorr signatures per BIP-340 / NIP-01. Co-Authored-By: claude-flow <ruv@ruv.net>
There was a problem hiding this comment.
Pull request overview
Removes an unused browser-side cryptography implementation (src/crypto-browser.js) that is insecure and could be accidentally picked up by future build/bundler configuration, ensuring the extension only relies on the established @noble/secp256k1-based implementation.
Changes:
- Deleted
src/crypto-browser.jsto eliminate a hazardous, non-standard “Schnorr” signing implementation. - Confirmed no remaining references/imports to
crypto-browser.jsin the repository (per search).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/crypto-browser.jswhich contains a completely broken cryptographic implementation that leaks private key material through signaturesSecurity Impact (CRITICAL)
crypto-browser.jsimplements a fake "Schnorr" signature scheme that is catastrophically broken:Public key derivation: Uses
SHA-256(private_key)instead of secp256k1 elliptic curve scalar multiplication. This means the public key is a simple hash of the private key rather than a computationally irreversible EC point.Signature construction: Creates signatures as
SHA-256(private_key || event_id)duplicated to fill 128 hex characters. Since event IDs are public knowledge, an attacker observing any signature hasSHA-256(private_key || known_value)-- this is a known-plaintext scenario that enables offline brute-force recovery of the private key.No actual Schnorr: Despite the filename, there is no elliptic curve operation anywhere in the file. It is pure SHA-256 hashing.
Current Risk
The file is not imported in the current bundled build --
src/background.jsimports fromsrc/crypto.jswhich correctly uses@noble/secp256k1. However, the file's presence in the source tree is an active hazard:package.json"browser"field mapping could silently redirectcrypto.jsimports tocrypto-browser.jsDeleting the file eliminates this risk entirely with zero impact on current functionality.
Test plan
src/background.jsimports from./crypto.js(not./crypto-browser.js) -- confirmedcrypto-browser.js-- confirmed via greppackage.jsonhas no"browser"field mapping -- confirmednpm run buildto confirm bundle still works