diff --git a/src/tests.c b/src/tests.c index 7edfa8a4fd369..2f2cb71539049 100644 --- a/src/tests.c +++ b/src/tests.c @@ -5166,6 +5166,21 @@ void run_ecdsa_openssl(void) { # include "modules/recovery/tests_impl.h" #endif +void run_memczero_test(void) { + unsigned char buf1[6] = {1, 2, 3, 4, 5, 6}; + unsigned char buf2[sizeof(buf1)]; + + /* memczero(..., ..., 0) is a noop. */ + memcpy(buf2, buf1, sizeof(buf1)); + memczero(buf1, sizeof(buf1), 0); + CHECK(memcmp(buf1, buf2, sizeof(buf1)) == 0); + + /* memczero(..., ..., 1) zeros the buffer. */ + memset(buf2, 0, sizeof(buf2)); + memczero(buf1, sizeof(buf1) , 1); + CHECK(memcmp(buf1, buf2, sizeof(buf1)) == 0); +} + int main(int argc, char **argv) { unsigned char seed16[16] = {0}; unsigned char run32[32] = {0}; @@ -5299,6 +5314,9 @@ int main(int argc, char **argv) { run_recovery_tests(); #endif + /* util tests */ + run_memczero_test(); + secp256k1_rand256(run32); printf("random run = %02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x\n", run32[0], run32[1], run32[2], run32[3], run32[4], run32[5], run32[6], run32[7], run32[8], run32[9], run32[10], run32[11], run32[12], run32[13], run32[14], run32[15]); diff --git a/src/util.h b/src/util.h index 2f96e2fc27d49..9a86e78757122 100644 --- a/src/util.h +++ b/src/util.h @@ -160,13 +160,16 @@ static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_siz SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t; #endif -/* Zero memory if flag == 1. Constant time. */ +/* Zero memory if flag == 1. Flag must be 0 or 1. Constant time. */ static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) { - unsigned char *p; - unsigned char mask = -(unsigned char)flag; - p = (unsigned char *)s; + unsigned char *p = (unsigned char *)s; + /* Access flag with a volatile-qualified lvalue. + This prevents clang from figuring out (after inlining) that flag can + take only be 0 or 1, which leads to variable time code. */ + volatile int vflag = flag; + unsigned char mask = -(unsigned char) vflag; while (len) { - *p ^= *p & mask; + *p &= ~mask; p++; len--; }