From f7297c27e3f6a440165443a9bc46e3fc41094150 Mon Sep 17 00:00:00 2001 From: Loup Vaillant Date: Sat, 14 Oct 2017 12:11:20 +0200 Subject: [PATCH] Fixed bogus comparison functions 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. --- src/monocypher.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/monocypher.c b/src/monocypher.c index d470693..8a37476 100644 --- a/src/monocypher.c +++ b/src/monocypher.c @@ -68,16 +68,16 @@ static u32 rotl32(u32 x, u32 n) { return (x << n) ^ (x >> (32 - n)); } 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); } -- 2.47.3