From 8872a97ebd81c9b1b649453664ee8a2dea9ceb2d Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 23 Jul 2019 18:40:09 +0100 Subject: [PATCH] gtkask.c: use dedicated PRNG for initial area choice. At Coverity's urging, I put in an instance of the Fortuna PRNG in place of the use of rand() to select which drawing area to light up next on a keypress. But Coverity turns out to be unhappy that I'm still using rand() to select the _initial_ area, before the user enters any input at all. It's even harder to imagine this being a genuine information leak than the previous complaint. But I don't really see a reason _not_ to switch it over, now it's been pointed out. --- unix/gtkask.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/unix/gtkask.c b/unix/gtkask.c index 064e3f18..6105a6a5 100644 --- a/unix/gtkask.c +++ b/unix/gtkask.c @@ -78,7 +78,7 @@ static void cleanup_keypress_prng(void) { prng_free(keypress_prng); } -static int choose_new_area(int prev_area) +static uint64_t keypress_prng_value(void) { /* * Don't actually put the passphrase keystrokes themselves into @@ -89,8 +89,11 @@ static int choose_new_area(int prev_area) noise_ultralight(NOISE_SOURCE_KEY, 0); uint8_t data[8]; prng_read(keypress_prng, data, 8); - uint64_t randval = GET_64BIT_MSB_FIRST(data); - int reduced = randval % (N_DRAWING_AREAS - 1); + return GET_64BIT_MSB_FIRST(data); +} +static int choose_new_area(int prev_area) +{ + int reduced = keypress_prng_value() % (N_DRAWING_AREAS - 1); return (prev_area + 1 + reduced) % N_DRAWING_AREAS; } @@ -353,7 +356,7 @@ static gboolean try_grab_keyboard(gpointer vctx) * And repaint the key-acknowledgment drawing areas as not greyed * out. */ - ctx->active_area = rand() % N_DRAWING_AREAS; + ctx->active_area = keypress_prng_value() % N_DRAWING_AREAS; for (i = 0; i < N_DRAWING_AREAS; i++) { ctx->drawingareas[i].state = (i == ctx->active_area ? CURRENT : NOT_CURRENT);