-
Notifications
You must be signed in to change notification settings - Fork 176
Description
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:
Lines 70 to 73 in 459b64f
hashByte := append(DBByte, VByte...) | |
hashByte = append(hashByte, RByte...) | |
hashByte = append(hashByte, proverLabel...) | |
hashByte = append(hashByte, verifierLabel...) |
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.