From 2c0a82e3e234d4263a32e759699ba64e4b56dac0 Mon Sep 17 00:00:00 2001
From: Michael Hope <mlhx@google.com>
Date: Tue, 26 Oct 2021 20:18:55 +0200
Subject: [PATCH] tftpd: fix the parsing of options

TFTP supports passing options such as 'blksize' as part of the initial
RRQ or WRQ packet. These options consist of a null-terminated name
followed by a null-terminated value. The current code segfaults due to
mis-calculating the address of the value.

Add a `next_token` helper that advances past the next null and also
detects advancing past the end of the packet.

Tested by running:

    atftp -g -r /toybox -l toybox --option "blksize 1500" --option "foo bar" localhost 8069

Signed-off-by: Michael Hope <mlhx@google.com>
---
 toys/pending/tftpd.c | 56 +++++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/toys/pending/tftpd.c b/toys/pending/tftpd.c
index f945aba6..b1ebb013 100644
--- a/toys/pending/tftpd.c
+++ b/toys/pending/tftpd.c
@@ -84,6 +84,21 @@ static void send_errpkt(struct sockaddr *dstaddr,
     perror_exit("sendto failed");
 }
 
+// Advance to the next option or value. Returns NULL if there are no
+// more options.
+static char *next_token(char *at, char *end)
+{
+  if (at == NULL) return NULL;
+
+  for (; at < end; at++) {
+    if (*at == '\0') {
+      at++;
+      break;
+    }
+  }
+  return (at < end) ? at : NULL;
+}
+
 // Used to send / receive packets.
 static void do_action(struct sockaddr *srcaddr, struct sockaddr *dstaddr,
     socklen_t socklen, char *file, int opcode, int tsize, int blksize)
@@ -208,7 +223,7 @@ POLL_INPUT:
         break;
       }
     }
-    
+
     // server will receive DATA pkt and write the data.
     if ((opcode == TFTPD_OP_WRQ) && (pktopcode == TFTPD_OP_DATA)) {
       if (rblockno == blockno) {
@@ -218,7 +233,7 @@ POLL_INPUT:
           send_errpkt(dstaddr, socklen, "write error");
           break;
         }
-      
+
         if (nw != blksize) done = 1;
       }
       continue;
@@ -236,10 +251,11 @@ CLEAN_APP:
 
 void tftpd_main(void)
 {
-  int fd = 0, recvmsg_len, rbuflen, opcode, blksize = TFTPD_BLKSIZE, tsize = 0, set =1;
+  int fd = 0, recvmsg_len, opcode, blksize = TFTPD_BLKSIZE, tsize = 0, set =1, bflag = 0;
   struct sockaddr_storage srcaddr, dstaddr;
   socklen_t socklen = sizeof(struct sockaddr_storage);
   char *buf = toybuf;
+  char *end;
 
   memset(&srcaddr, 0, sizeof(srcaddr));
   if (getsockname(0, (struct sockaddr *)&srcaddr, &socklen)) help_exit(0);
@@ -248,6 +264,7 @@ void tftpd_main(void)
   if (*toys.optargs) xchroot(*toys.optargs);
 
   recvmsg_len = recvfrom(fd, toybuf, blksize, 0, (void *)&dstaddr, &socklen);
+  end = toybuf + recvmsg_len;
 
   TT.sfd = xsocket(dstaddr.ss_family, SOCK_DGRAM, 0);
   if (setsockopt(TT.sfd, SOL_SOCKET, SO_REUSEADDR, (const void *)&set,
@@ -275,32 +292,29 @@ void tftpd_main(void)
     return;
   }
 
-  buf += strlen(buf) + 1; //1 '\0'.
+  buf = next_token(buf, end);
   // As per RFC 1350, mode is case in-sensitive.
-  if (buf >= toybuf+recvmsg_len || strcasecmp(buf, "octet")) {
+  if (buf == NULL || strcasecmp(buf, "octet")) {
     send_errpkt((struct sockaddr*)&dstaddr, socklen, "packet format error");
     return;
   }
 
   //RFC2348. e.g. of size type: "ttype1\0ttype1_val\0...ttypeN\0ttypeN_val\0"
-  buf += strlen(buf) + 1;
-  rbuflen = toybuf + recvmsg_len - buf;
-  if (rbuflen) {
-    int jump = 0, bflag = 0;
-
-    for (; rbuflen; rbuflen -= jump, buf += jump) {
-      if (!bflag && !strcasecmp(buf, "blksize")) { //get blksize
-        errno = 0;
-        blksize = strtoul(buf, NULL, 10);
-        if (errno || blksize > 65564 || blksize < 8) blksize = TFTPD_BLKSIZE;
-        bflag ^= 1;
-      } else if (!tsize && !strcasecmp(buf, "tsize")) tsize ^= 1;
-      
-      jump += strlen(buf) + 1;
-    }
-    tsize &= (opcode == TFTPD_OP_RRQ);
+  for (buf = next_token(buf, end); buf != NULL; buf = next_token(buf, end)) {
+    char *opt = buf;
+    buf = next_token(buf, end);
+    if (buf == NULL) break; // Missing value.
+
+    if (!bflag && !strcasecmp(opt, "blksize")) {
+      errno = 0;
+      blksize = strtoul(buf, NULL, 10);
+      if (errno || blksize > 65564 || blksize < 8) blksize = TFTPD_BLKSIZE;
+      bflag ^= 1;
+    } else if (!tsize && !strcasecmp(opt, "tsize")) tsize ^= 1;
   }
 
+  tsize &= (opcode == TFTPD_OP_RRQ);
+
   //do send / receive file.
   do_action((struct sockaddr*)&srcaddr, (struct sockaddr*)&dstaddr,
       socklen, toybuf + 2, opcode, tsize, blksize);
-- 
2.39.2