| 123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120 |
- From 8284fca0f096d01f566eadfdc790232df9f2934e Mon Sep 17 00:00:00 2001
- From: B Horn <b@horn.uk>
- Date: Thu, 18 Apr 2024 17:32:34 +0100
- Subject: [PATCH] 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>
- Upstream: 0707accab1b9be5d3645d4700dde3f99209f9367
- Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.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.50.1
|