Found by michaelforney on Github.
- On neq0 , I used ^ instead of |
- On zerocmp32, I used + instead of |
Both errors lead to false negatives: you *think* all went well and the
number looks like it is indeed, zero, but it's not. This could lead
to vulnerabilities in practice, where we could use the flaws in the
comparison functions to find pseudo-collisions, that defeat the checks
without being actual collisions.
Oops.
static int neq0(u64 diff)
{ // constant time comparison to zero
// return diff != 0 ? -1 : 0
- u64 half = (diff >> 32) ^ ((u32)diff);
+ u64 half = (diff >> 32) | ((u32)diff);
return (1 & ((half - 1) >> 32)) - 1;
}
-int zerocmp32(const u8 p[32])
+static int zerocmp32(const u8 p[32])
{
u64 all = load64_le(p + 0)
- + load64_le(p + 8)
- + load64_le(p + 16)
- + load64_le(p + 24);
+ | load64_le(p + 8)
+ | load64_le(p + 16)
+ | load64_le(p + 24);
return neq0(all);
}