]> git.codecow.com Git - Monocypher.git/commitdiff
Fixed bogus comparison functions
authorLoup Vaillant <loup@loup-vaillant.fr>
Sat, 14 Oct 2017 10:11:20 +0000 (12:11 +0200)
committerLoup Vaillant <loup@loup-vaillant.fr>
Sat, 14 Oct 2017 10:11:20 +0000 (12:11 +0200)
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

index d470693865fb9f587fc1143a08e822303efa7a8e..8a37476cfb398f725ce5f94525778f93362059af 100644 (file)
@@ -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);
 }