Browse Source

Coverity: different implementation for csiphash

Coverity has had trouble figuring out our csiphash implementation,
and has given spurious warnings about its behavior.

This patch changes the csiphash implementation when coverity is in
use, so that coverity can figure out that we are not about to read
beyond the provided input.

Closes ticket 31025.
Nick Mathewson 4 years ago
parent
commit
f55598f870
2 changed files with 13 additions and 0 deletions
  1. 5 0
      changes/ticket31025
  2. 8 0
      src/ext/csiphash.c

+ 5 - 0
changes/ticket31025

@@ -0,0 +1,5 @@
+  o Minor bugfixes (coverity):
+    - In our siphash implementation, when building for coverity, use memcpy
+      in place of a switch statement, so that coverity can tell we are not
+      accessing out-of-bounds memory. Fixes bug 31025; bugfix on
+      0.2.8.1-alpha.  This is tracked as CID 1447293 and 1447295.

+ 8 - 0
src/ext/csiphash.c

@@ -87,6 +87,13 @@ uint64_t siphash24(const void *src, unsigned long src_sz, const struct sipkey *k
 		v0 ^= mi;
 		v0 ^= mi;
 	}
 	}
 
 
+#ifdef __COVERITY__
+	{
+		uint64_t mi = 0;
+		memcpy(&mi, m+i, (src_sz-blocks));
+		last7 = _le64toh(mi) | (uint64_t)(src_sz & 0xff) << 56;
+	}
+#else
 	switch (src_sz - blocks) {
 	switch (src_sz - blocks) {
 		case 7: last7 |= (uint64_t)m[i + 6] << 48; /* Falls through. */
 		case 7: last7 |= (uint64_t)m[i + 6] << 48; /* Falls through. */
 		case 6: last7 |= (uint64_t)m[i + 5] << 40; /* Falls through. */
 		case 6: last7 |= (uint64_t)m[i + 5] << 40; /* Falls through. */
@@ -98,6 +105,7 @@ uint64_t siphash24(const void *src, unsigned long src_sz, const struct sipkey *k
 		case 0:
 		case 0:
 		default:;
 		default:;
 	}
 	}
+#endif
 	v3 ^= last7;
 	v3 ^= last7;
 	DOUBLE_ROUND(v0,v1,v2,v3);
 	DOUBLE_ROUND(v0,v1,v2,v3);
 	v0 ^= last7;
 	v0 ^= last7;