From 0707accab1b9be5d3645d4700dde3f99209f9367 Mon Sep 17 00:00:00 2001
From: B Horn <b@horn.uk>
Date: Thu, 18 Apr 2024 17:32:34 +0100
Subject: [PATCH 31/73] net/tftp: Fix stack buffer overflow in tftp_open()

An overly long filename can be passed to tftp_open() which would cause
grub_normalize_filename() to write out of bounds.

Fixed by adding an extra argument to grub_normalize_filename() for the
space available, making it act closer to a strlcpy(). As several fixed
strings are strcpy()'d after into the same buffer, their total length is
checked to see if they exceed the remaining space in the buffer. If so,
return an error.

On the occasion simplify code a bit by removing unneeded rrqlen zeroing.

Reported-by: B Horn <b@horn.uk>
Signed-off-by: B Horn <b@horn.uk>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 grub-core/net/tftp.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
index 409b1d09b..336b78691 100644
--- a/grub-core/net/tftp.c
+++ b/grub-core/net/tftp.c
@@ -266,17 +266,19 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
  * forward slashes to a single forward slash.
  */
 static void
-grub_normalize_filename (char *normalized, const char *filename)
+grub_normalize_filename (char *normalized, const char *filename, int c)
 {
   char *dest = normalized;
   const char *src = filename;
 
-  while (*src != '\0')
+  while (*src != '\0' && c > 0)
     {
       if (src[0] == '/' && src[1] == '/')
         src++;
-      else
+      else {
+        c--;
         *dest++ = *src++;
+      }
     }
   *dest = '\0';
 }
@@ -287,7 +289,7 @@ tftp_open (struct grub_file *file, const char *filename)
   struct tftphdr *tftph;
   char *rrq;
   int i;
-  int rrqlen;
+  int rrqlen, rrqsize;
   int hdrlen;
   grub_uint8_t open_data[1500];
   struct grub_net_buff nb;
@@ -315,37 +317,45 @@ tftp_open (struct grub_file *file, const char *filename)
 
   tftph = (struct tftphdr *) nb.data;
 
-  rrq = (char *) tftph->u.rrq;
-  rrqlen = 0;
-
   tftph->opcode = grub_cpu_to_be16_compile_time (TFTP_RRQ);
 
+  rrq = (char *) tftph->u.rrq;
+  rrqsize = sizeof (tftph->u.rrq);
+
   /*
    * Copy and normalize the filename to work-around issues on some TFTP
    * servers when file names are being matched for remapping.
    */
-  grub_normalize_filename (rrq, filename);
-  rrqlen += grub_strlen (rrq) + 1;
+  grub_normalize_filename (rrq, filename, rrqsize);
+
+  rrqlen = grub_strlen (rrq) + 1;
   rrq += grub_strlen (rrq) + 1;
 
-  grub_strcpy (rrq, "octet");
+  /* Verify there is enough space for the remaining components. */
   rrqlen += grub_strlen ("octet") + 1;
+  rrqlen += grub_strlen ("blksize") + 1;
+  rrqlen += grub_strlen ("1024") + 1;
+  rrqlen += grub_strlen ("tsize") + 1;
+  rrqlen += grub_strlen ("0") + 1;
+
+  if (rrqlen >= rrqsize) {
+    grub_free (data);
+    return grub_error (GRUB_ERR_BAD_FILENAME, N_("filename too long"));
+  }
+
+  grub_strcpy (rrq, "octet");
   rrq += grub_strlen ("octet") + 1;
 
   grub_strcpy (rrq, "blksize");
-  rrqlen += grub_strlen ("blksize") + 1;
   rrq += grub_strlen ("blksize") + 1;
 
   grub_strcpy (rrq, "1024");
-  rrqlen += grub_strlen ("1024") + 1;
   rrq += grub_strlen ("1024") + 1;
 
   grub_strcpy (rrq, "tsize");
-  rrqlen += grub_strlen ("tsize") + 1;
   rrq += grub_strlen ("tsize") + 1;
 
   grub_strcpy (rrq, "0");
-  rrqlen += grub_strlen ("0") + 1;
   rrq += grub_strlen ("0") + 1;
   hdrlen = sizeof (tftph->opcode) + rrqlen;
 
-- 
2.33.0

