Fixes: Add digest length checking #405
Conversation
Signed-off-by: Arthur Chan <[email protected]>
| if (artifactDigest.length > 64) { | ||
| throw new SignatureExeption("Artifact digest cannot be longer than 64 bytes for this mode"); | ||
| } |
There was a problem hiding this comment.
How is this better than the exception from signature.update(artifactDigest); ?
There was a problem hiding this comment.
I wonder if the check should be implemented in the downstream library instead. Of course, it is easier to sweep it under the rug here, however, it is hard to understand why we have an artificial limit of 64 here.
So I would suggest filing an issue/PR for the impl library rather than sigstore-java
There was a problem hiding this comment.
That makes sense to me. Let's make a PR there and see how it's retrieved.
There was a problem hiding this comment.
Just checking in? Was there any resolution on this? Is there a PR somewhere to follow up on?
There was a problem hiding this comment.
Ah, I missed this! Apologies -- @arthurscchan could you help with this one?
There was a problem hiding this comment.
I just spoke to @arthurscchan and @AdamKorcz who said it may not be the correct choice to change upstream. @AdamKorcz could you elaborate?
There was a problem hiding this comment.
The best "upstream" place to fix this issue is at the provider level. However, this is not a suitable place to fix this issue, since there are many providers that sigstore-java users can consume. In addition, sigstore-java users can write their own providers. As such, sigstore-java is the best place to do the digest length check.
|
ugh sorry, not all these statuses are manually triggerable. Can you rebase it so we can get all the statuses green? |
|
looks like this needs some manual intervention |
Signed-off-by: Arthur Chan <[email protected]>
According to the ECDSASignature class, the NoneWithECDSA mode only accept a digest with maximum length of 64 bytes, using a digest longer than 64 will result in ArrayIndexOutOfBoundException when the full byte array is copied to a byte array with only 64 bytes.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=57360
Summary
Release Note
Documentation