From 73e81dc78ddb6f1bc0d8750c1a9e19e6cc123d48 Mon Sep 17 00:00:00 2001 From: Vladis Dronov Date: Sat, 12 Jun 2021 09:16:25 +0200 Subject: Fix logic in ossl_aes_random_key() - Using sizeof is correct for arr[], but is not correct for *ptr. read() here makes a 8-bytes read (on 64-bit arch). It should read AES_BLOCK bytes instead, as callers of ossl_aes_random_key() imply. Covscan emits the following warning: Error: SIZEOF_MISMATCH (CWE-398): [#def1] rng-tools-6.12/ossl_helpers.c:51: suspicious_sizeof: Passing argument "key" of type "unsigned char *" and argument "8UL /* sizeof (key) */" to function "read" is suspicious. 51|-> int r = read(fd, key, sizeof key); - According to the following warning, mark r as not used explicitly: Error: CLANG_WARNING: [#def2] rng-tools-6.12/ossl_helpers.c:51:7: warning[deadcode.DeadStores]: Value stored to 'r' during its initialization is never read 51|-> int r = read(fd, key, sizeof key); - Add volatile to stack_junk to avoid possible compiler optimization. This does not silence "Uninitialized variable: stack_junk" covscan warning. - Remove a check for pepper == NULL where it is not needed. Signed-off-by: Vladis Dronov --- ossl_helpers.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ossl_helpers.c b/ossl_helpers.c index c3c1fbb..9569b74 100644 --- a/ossl_helpers.c +++ b/ossl_helpers.c @@ -42,22 +42,23 @@ void ossl_aes_random_key(unsigned char *key, const unsigned char *pepper) 0x00,0x10,0x20,0x30,0x40,0x50,0x60,0x70, 0x80,0x90,0xa0,0xb0,0xc0,0xd0,0xe0,0xf0 }; /* AES data reduction key */ - unsigned char stack_junk[AES_BLOCK]; + volatile unsigned char stack_junk[AES_BLOCK]; int fd, i; /* Try getting some randomness from the kernel */ fd = open("/dev/urandom", O_RDONLY); if (fd >= 0) { - int r = read(fd, key, sizeof key); + int r __attribute__((unused)); + r = read(fd, key, AES_BLOCK); close(fd); } /* Mix in our default key */ - for (i = 0; i < AES_BLOCK && pepper; i++) + for (i = 0; i < AES_BLOCK; i++) key[i] ^= default_key[i]; /* Mix in stack junk */ - for (i = 0; i < AES_BLOCK && pepper; i++) + for (i = 0; i < AES_BLOCK; i++) key[i] ^= stack_junk[i]; /* Spice it up if we can */ -- 2.26.3