From 31a4ba72646de34d26923e2293a1ea08f52e9f7f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 17 Jun 2019 15:01:08 +0200 Subject: [PATCH] Fix misuse of signed ints in the HAVEGE module The elements of the HAVEGE state are manipulated with bitwise operations, with the expectations that the elements are 32-bit unsigned integers (or larger). But they are declared as int, and so the code has undefined behavior. Clang with Asan correctly points out some shifts that reach the sign bit. Use unsigned int internally. This is technically an aliasing violation since we're accessing an array of `int` via a pointer to `unsigned int`, but since we don't access the array directly inside the same function, it's very unlikely to be compiled in an unintended manner. --- library/havege.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/library/havege.c b/library/havege.c index 54f897c6e..08f9d974d 100644 --- a/library/havege.c +++ b/library/havege.c @@ -54,7 +54,7 @@ * ------------------------------------------------------------------------ */ -#define SWAP(X,Y) { int *T = (X); (X) = (Y); (Y) = T; } +#define SWAP(X,Y) { unsigned *T = (X); (X) = (Y); (Y) = T; } #define TST1_ENTER if( PTEST & 1 ) { PTEST ^= 3; PTEST >>= 1; #define TST2_ENTER if( PTEST & 1 ) { PTEST ^= 3; PTEST >>= 1; @@ -77,7 +77,7 @@ PTX = (PT1 >> 18) & 7; \ PT1 &= 0x1FFF; \ PT2 &= 0x1FFF; \ - CLK = (int) mbedtls_timing_hardclock(); \ + CLK = (unsigned) mbedtls_timing_hardclock(); \ \ i = 0; \ A = &WALK[PT1 ]; RES[i++] ^= *A; \ @@ -100,7 +100,7 @@ \ IN = (*A >> (5)) ^ (*A << (27)) ^ CLK; \ *A = (*B >> (6)) ^ (*B << (26)) ^ CLK; \ - *B = IN; CLK = (int) mbedtls_timing_hardclock(); \ + *B = IN; CLK = (unsigned) mbedtls_timing_hardclock(); \ *C = (*C >> (7)) ^ (*C << (25)) ^ CLK; \ *D = (*D >> (8)) ^ (*D << (24)) ^ CLK; \ \ @@ -151,19 +151,20 @@ PT1 ^= (PT2 ^ 0x10) & 0x10; \ \ for( n++, i = 0; i < 16; i++ ) \ - hs->pool[n % MBEDTLS_HAVEGE_COLLECT_SIZE] ^= RES[i]; + POOL[n % MBEDTLS_HAVEGE_COLLECT_SIZE] ^= RES[i]; /* * Entropy gathering function */ static void havege_fill( mbedtls_havege_state *hs ) { - int i, n = 0; - int U1, U2, *A, *B, *C, *D; - int PT1, PT2, *WALK, RES[16]; - int PTX, PTY, CLK, PTEST, IN; + unsigned i, n = 0; + unsigned U1, U2, *A, *B, *C, *D; + unsigned PT1, PT2, *WALK, *POOL, RES[16]; + unsigned PTX, PTY, CLK, PTEST, IN; - WALK = hs->WALK; + WALK = (unsigned *) hs->WALK; + POOL = (unsigned *) hs->pool; PT1 = hs->PT1; PT2 = hs->PT2;