From a32d1a270438c0c8be235327b4ef7047edb4e60e Mon Sep 17 00:00:00 2001
From: Michael Jones <jonesmz@users.noreply.github.com>
Date: Mon, 27 May 2024 12:56:12 -0500
Subject: [PATCH] Avoid writing potentially uninitialized data to aes_128 key
 file (#1500)

---
 src/apps/relay/mainrelay.c | 40 +++++++++++++++++++++++---------------
 src/apps/relay/mainrelay.h |  1 -
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/src/apps/relay/mainrelay.c b/src/apps/relay/mainrelay.c
index 5aaf9243..cf370ec8 100644
--- a/src/apps/relay/mainrelay.c
+++ b/src/apps/relay/mainrelay.c
@@ -1740,32 +1740,40 @@ void encrypt_aes_128(unsigned char *in, const unsigned char *mykey) {
   unsigned char *base64_encoded = base64encode(total, totalSize);
   printf("%s\n", base64_encoded);
 }
-void generate_aes_128_key(char *filePath, unsigned char *returnedKey) {
-  int i;
-  int part;
-  FILE *fptr;
+static void generate_aes_128_key(char *filePath, unsigned char *returnedKey) {
   char key[16];
+
+  // TODO: Document why this is called...?
   turn_srandom();
 
-  for (i = 0; i < 16; i++) {
-    part = (rand() % 3);
-    if (part == 0) {
+  for (size_t i = 0; i < 16; ++i) {
+    // TODO: This could be sped up by breaking the
+    // returned random value into multiple 8bit values
+    // instead of getting a new multi-byte random value
+    // for each key index.
+    switch (turn_random() % 3) {
+    case 0:
       key[i] = (turn_random() % 10) + 48;
-    }
-
-    else if (part == 1) {
+      continue;
+    case 1:
       key[i] = (turn_random() % 26) + 65;
-    }
-
-    else if (part == 2) {
+      continue;
+    default:
       key[i] = (turn_random() % 26) + 97;
+      continue;
     }
   }
-  fptr = fopen(filePath, "w");
-  for (i = 0; i < 16; i++) {
+  FILE *fptr = fopen(filePath, "w");
+  if (!fptr) {
+    return;
+  }
+  for (size_t i = 0; i < 16; ++i) {
     fputc(key[i], fptr);
   }
-  STRCPY((char *)returnedKey, key);
+  memcpy(returnedKey, key, 16);
+  // Note: Don't put a nul-terminator at the end.
+  // this function is only ever called with returnedKey
+  // as fixed size char arrays of size 16.
   fclose(fptr);
 }
 
diff --git a/src/apps/relay/mainrelay.h b/src/apps/relay/mainrelay.h
index 35a0d92d..ae781140 100644
--- a/src/apps/relay/mainrelay.h
+++ b/src/apps/relay/mainrelay.h
@@ -393,7 +393,6 @@ struct ctr_state {
   unsigned int num;
   unsigned char ecount[16];
 };
-void generate_aes_128_key(char *filePath, unsigned char *returnedKey);
 unsigned char *base64encode(const void *b64_encode_this, int encode_this_many_bytes);
 void encrypt_aes_128(unsigned char *in, const unsigned char *mykey);
 unsigned char *base64decode(const void *b64_decode_this, int decode_this_many_bytes);