-
Notifications
You must be signed in to change notification settings - Fork 466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CROSS broken sigs validate #1995
Comments
Hi @rtjk, this is correct, the issue occurred when incrementing a single random byte of the signature. |
The arrays that store nodes for the merkle tree and seed tree are fixed in size to a certain upper bound. However, they often end up with a bunch of empty (zero-padded) bytes at the end. It appears these zeros aren't checked during verification, so tampering with them results in a (wrongly) valid signature. Adding a check for these bytes should be straightforward, I’ll discuss this with the team and prepare a PR. Kudos to the liboqs test framework for catching this! By the way, this issue is somewhat related to #1958. |
I can confirm what @rtjk said before: the mismatches are arising from the fact that a specific technique (seed trees and Merkle trees) allow to save some signature size but, in turn have a variable length signature. The savings in signature size To avoid the pains of handling an actual variable-length specification from an engineering standpoint (this is especially troublesome in hardware), in CROSS we're allocating the maximum signature size and zero-padding the unused bytes. We did not check that the padding bytes are actually set to zero (as they are not used in the signature verification process, but, it's a good idea to check them anyway (strictly speaking, it's an EUF-CMA violation, although I cannot see how an attacker can profit from this on the spot). |
I’ve run the suggested test by @baentsch and noticed that the CROSS signature scheme seems to fail in some cases, passing the verification of modified signatures.
Tagging @rtjk.
Originally posted by @bhess in #1919 (comment)
The text was updated successfully, but these errors were encountered: