search

cloudflare / circl

CIRCL: Cloudflare Interoperable Reusable Cryptographic Library
http://blog.cloudflare.com/introducing-circl
Other
1.23k stars 136 forks source link

Set boundary between concatenated values in zk/dl #390

Closed sander closed 1 year ago

sander commented 1 year ago

RFC 8235 §2.3 says:

   Within the hash function, there must be a clear boundary between any
   two concatenated items.  It is RECOMMENDED that one should always
   prepend each item with a 4-byte integer that represents the byte
   length of that item.  OtherInfo may contain multiple subitems.  In
   that case, the same rule shall apply to ensure a clear boundary
   between adjacent subitems.

This is not happening in:

https://github.com/cloudflare/circl/blob/459b64f53bc65c6ef18923652e59d8d28da75057/zk/dl/dl.go#L70-L73

This potentially introduces vulnerabilities. For example, a proof with proverLabel = 0x01 and verifierLabel = 0x02 0x03 would now also be verified successfully with proverLabel = 0x01 0x02 and verifierLabel = 0x03, even if the prover did not intend that.

In practice I don’t expect any deployments to be vulnerable, since usually the labels would contain some structured data. Still this seems like a good recommendation to follow, to make it more difficult for CIRCL users to introduce vulnerabilities.

A fix would be to add lines such as:

    hashByte = binary.LittleEndian.AppendUint32(hashByte, uint32(len(proverLabel)))

Note that this would be a breaking change: existing proofs generated with the old implementation would fail to verify in the changed implementation.

armfazh commented 1 year ago

Hi @sander , it makes sense adding prefixed labels. I'll fix this soon.