Description

The circuits contain and verify a Schnorr signature of the relevant data (such as input and output notes) for the action, as in this snippet from the join circuit.

let m = std::hash::mimc::mimc_bn254([
    in_note_1,
    in_note_2,
    in_nullifier_1,
    in_nullifier_2,
    out_note_1,
]);
let m_bytes = fuzk::to_bytes(m);

let v = std::schnorr::verify_signature(
    pub_key[0],
    pub_key[1],
    signature,
    m_bytes,
);

assert(v);

The data that is signed does not however contain a domain separator that specifies which action is being signed. This means that an attacker (for example, a service constructing the relevant proof as a service to the user) that obtains one such signature might be able to use it to construct a proof for a different action than the user intended.

Doing so would require the length of the data being hashed to be equal between these two circuits. There are some clashes like this that could be used:

4:
split: [Field; 4] : [note, nullifier, out note, out note]
uniswap_swap: [Field; 4]: [note, nullifier, out footer, out asset]
transfer: [Field; 4]: [note, nullifier, out note, out footer]

5:
join: [Field; 5]: [note, note, nullifier, nullifier, out note]
curve_exchange: [Field; 5]: [note, nullifier, pool, out footer, out asset]
uniswap_collect_fees: [Field; 5]: [pos note, pos addr, pos token, note footer, note footer]
curve_multi_exchange: [Field; 5]: [note, nullifier, routeHash, out footer, out asset]

6:
join_split: [Field; 6]: [note, note, nullifier, nullifier, out note, out note]
swap: [Field; 6]: [note, note, nullifier, nullifier, out note, out note]
uniswap_remove_liquidity: [Field; 6]: [note, address, token, nullifier, out footer, out footer]

As the attacker would not be able to control the data being signed, attempting to prove the "wrong" circuit might mean to have to (say using a curve_exchange signature for join) prove that something is a well-formed note commitment, even though that piece of data was constructed as a nullifier of another note, or having to prove that something is a nullifier for a specific note even though it arose from a completely different piece of data. For many of the pairs above, it will thus be impossible in practice to use a signature for the wrong circuit. In some cases it might be possible, such as using a join_split signature for a swap proof. We have not found a way for an attacker to profit from any such proof for an incorrect circuit, however.

Note that, should Schnorr signatures of mimc_bn254 hashed data be used elsewhere as well (i.e., not just in these circuits), then possible issues of using such signatures elsewhere, or using such other signatures within the circuit, could arise as well.

Impact

The signatures in the circuits are not domain separated, so it is not ensured that such a signature was intended by the signer to be used for this particular circuit. No practical impact was identified at this time, but we still recommend proper domain separation.

Recommendations

Add a domain separator to the data to be signed. This domain separator should be a different constant for each circuit, and it should be unlikely that the same domain separator is used accidentally for a different purpose.

For example, as the domain separator for the join circuit, one might use DOMAIN_SEPARATOR_JOIN=mimc_bn254("SINGULARITY_CIRCUIT_JOIN") (pseudocode, the string would need to be cast to an array of field elements) as a domain separator and add that constant as a prefix to the data to be signed:

let m = std::hash::mimc::mimc_bn254([
+     DOMAIN_SEPARATOR_JOIN,
    in_note_1,
    in_note_2,
    in_nullifier_1,
    in_nullifier_2,
    out_note_1,
]);
let m_bytes = fuzk::to_bytes(m);

let v = std::schnorr::verify_signature(
    pub_key[0],
    pub_key[1],
    signature,
    m_bytes,
);

assert(v);

Remediation

This issue has been acknowledged by Singularity and a fix for all circuits except swap was implemented in commit . The swap entrypoint was disabled in commit .