urgent security fix: digest verification#14
Conversation
junkurihara
left a comment
junkurihara
left a comment
There was a problem hiding this comment.
Thank you very much for reporting this and for the patch.
This PR fixes the critical issue where digest verification could incorrectly succeed due to incorrect use of matches!, which could cause verification to always return true.
As a follow-up, I will switch the digest comparison to a constant-time comparison as defense-in-depth, and add regression tests to ensure this class of bug cannot be reintroduced. The next release will include the full fix together with the additional hardening and tests. It will be released shortly.
For future security-related findings, it would be helpful to use GitHub Security Advisories (Security tab -> "Report a vulnerability"), so we can coordinate fixes privately before public discussion.
Thanks again for the report and contribution.
| let digest = derive_digest(&body_bytes, &cd_type); | ||
|
|
||
| if matches!(digest, _expected_digest) { | ||
| if digest == _expected_digest { |
There was a problem hiding this comment.
In the next release, I will switch the digest comparison to a constant-time comparison as defense-in-depth, and add regression tests to ensure this class of bug cannot be reintroduced.
| let digest = derive_digest(&body_bytes, &cd_type); | ||
|
|
||
| if matches!(digest, _expected_digest) { | ||
| if digest == _expected_digest { |
|
agreed. I also sent u a linkedin invite to discuss security patches privately there as well (I hope we won't face any of course) |
|
Just published GHSA GHSA-7v42-g35v-xrch |
2 participants
Hello again,
the previous variant used matches!(Vec, Vec), which for vectors always returns true. this can potentially allow attackers to reuse a captured request with custom body.
the problem has been confirmed in our project, I was able to freely modify request body after the digest and signature had been placed and the server still accepted it.