Add cryptography not-so-smart-contracts section#425
Conversation
…entries Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
omarinuwa
requested review from
Jaime-Iglesias,
anishnaik and
bsamuels453
as code owners
|
|
jvdprng
left a comment
jvdprng
left a comment
There was a problem hiding this comment.
Nice work! Some comments with potential improvements.
|
|
||
| Distributed key generation (DKG) and multi-party computation (MPC) protocols coordinate multiple participants through structured message rounds to jointly generate keys or compute signatures without any single party holding the full secret. Because these protocols involve complex multi-round state machines, subtle implementation bugs can break the security model entirely -- even when the underlying cryptographic primitives are correct. | ||
|
|
||
| Common flaws include failing to validate the degree of shared polynomials in Feldman VSS, allowing a malicious participant to silently raise the reconstruction threshold; poor abort handling that lets an adversary leak secret bits through selective failures or deny service by triggering unilateral aborts; and reuse of session identifiers that enables cross-session message replay, leading to state corruption or key compromise. |
There was a problem hiding this comment.
One thing I would add: the length check also matters for pretty much anything else exchanged in a DKG/MPC protocol. Frequently there are issues where the code assumes that something has a certain length and may panic when you send something shorter. The Feldman VSS is a specific case where it allows you to increase the threshold, but length checks are needed everywhere.
Another pattern we often see is that DKG / MPC protocols require you to verify a number of ZK proofs for certain values, but the code doesn't properly check the lengths. Then, an attacker can send an empty list of proofs and (e.g. due to Rust's zip) the code will just not process any proofs, not triggering any error in proof verification.
| - Implement blame and identify protocols so that abort handling attributes the fault to a specific participant rather than allowing silent denial of service | ||
| - Generate unique session IDs bound to the participant set, round number, and fresh randomness; reject any message whose session tag does not match the current session | ||
| - Authenticate all protocol messages (e.g., with sender signatures) before parsing or processing them, preventing replay and injection attacks | ||
| - In oblivious transfer sub-protocols, ensure cheating detection identifies the specific malicious party rather than panicking, to prevent selective abort attacks that leak secret bits across sessions |
There was a problem hiding this comment.
Preventing selective abort attacks becomes infinitely harder if the protocol allows multiple sessions to be executed in parallel, because you need to trigger an abort instantly across all sessions. The more realistic mitigation here is to perform fresh base OTs for each protocol execution (i.e., do not reuse them for OT extension across executions).
| # Hash Domain Separation Failure | ||
|
|
||
| Hash functions should produce different outputs for semantically different inputs, even | ||
| when the raw byte representations happen to be identical. This property is called domain |
There was a problem hiding this comment.
I would correct here that with domain separation it is impossible to get identical raw byte representations of the result for semantically different inputs. That is in fact the goal of domain separation.
|
|
||
| ## Mitigations | ||
|
|
||
| - **Include the input length** as an explicit field in every hash invocation |
There was a problem hiding this comment.
Make it clear that this is both for the overall input length, but also for the individual lengths of each subitem in the input. E.g., if I have two inputs "0000" and "000000", the result should be something like "0702000003000000". Otherwise, "050000000000" may still allow confusion between the pairs ("0000", "000000") and ("000000", "0000").
| The same class of bug applies to AEAD channels. If an MPC node encrypts two | ||
| different round messages under the same `(shared_key, nonce)` pair, an | ||
| observer can XOR the two ciphertexts to cancel the keystream and obtain the | ||
| XOR of the two plaintexts, from which both can often be recovered. |
There was a problem hiding this comment.
I'm not sure if this is true. In MPC protocols you may often end up with the XOR of two random values, which are difficult to recover without additional information. However, in some cases there are attacks so it's definitely worth mitigating.
| } | ||
| ``` | ||
|
|
||
| ## Mitigations |
There was a problem hiding this comment.
Maybe worth noting that this is very hard to mitigate completely, because you're at the mercy of the compiler and the CPU in many cases.
… blocks - Add ## Description and ## Exploit Scenario sections to all entries - Add one-line summaries after H1 titles - Remove large fixed code blocks exceeding 2-5 line limit - Remove forbidden VULNERABLE/FIXED/FIX labels from code blocks - Add language tags to untagged code blocks - Fix ecosystem README template compliance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…code - Remove cryptography section (belongs to PR crytic#425) - Add ## Description and ## Exploit Scenario sections to all Circom entries - Add one-line summaries after H1 titles - Remove large fixed code blocks exceeding 2-5 line limit - Remove forbidden // FIX: labels from code blocks - Expand missing_input_constraints code to meet 10-line minimum Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adjust markdown table column widths to match Prettier's expected output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4 participants
Summary
New Entries
Test plan
🤖 Generated with Claude Code