From c8665ebbdadb72f7c2cb74b9704f68704c13653b Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@redhat.com>

Date: Fri, 4 Jun 2021 20:01:20 +0400

Subject: [PATCH 6/7] tftp: introduce a header structure

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Instead of using a composed structure and potentially reading past the

incoming buffer, use a different structure for the header.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

(cherry picked from commit 990163cf3ac86b7875559f49602c4d76f46f6f30)

---

 src/tftp.c | 60 ++++++++++++++++++++++++++++--------------------------

 src/tftp.h |  6 +++++-

 2 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/src/tftp.c b/src/tftp.c

index e06911d..a19c889 100644

--- a/src/tftp.c

+++ b/src/tftp.c

@@ -50,7 +50,7 @@ static void tftp_session_terminate(struct tftp_session *spt)

 }

 static int tftp_session_allocate(Slirp *slirp, struct sockaddr_storage *srcsas,

-                                 struct tftp_t *tp)

+                                 struct tftphdr *hdr)

 {

     struct tftp_session *spt;

     int k;

@@ -75,7 +75,7 @@ found:

     memcpy(&spt->client_addr, srcsas, sockaddr_size(srcsas));

     spt->fd = -1;

     spt->block_size = 512;

-    spt->client_port = tp->udp.uh_sport;

+    spt->client_port = hdr->udp.uh_sport;

     spt->slirp = slirp;

     tftp_session_update(spt);

@@ -84,7 +84,7 @@ found:

 }

 static int tftp_session_find(Slirp *slirp, struct sockaddr_storage *srcsas,

-                             struct tftp_t *tp)

+                             struct tftphdr *hdr)

 {

     struct tftp_session *spt;

     int k;

@@ -94,7 +94,7 @@ static int tftp_session_find(Slirp *slirp, struct sockaddr_storage *srcsas,

         if (tftp_session_in_use(spt)) {

             if (sockaddr_equal(&spt->client_addr, srcsas)) {

-                if (spt->client_port == tp->udp.uh_sport) {

+                if (spt->client_port == hdr->udp.uh_sport) {

                     return k;

                 }

             }

@@ -148,13 +148,13 @@ static struct tftp_t *tftp_prep_mbuf_data(struct tftp_session *spt,

 }

 static void tftp_udp_output(struct tftp_session *spt, struct mbuf *m,

-                            struct tftp_t *recv_tp)

+                            struct tftphdr *hdr)

 {

     if (spt->client_addr.ss_family == AF_INET6) {

         struct sockaddr_in6 sa6, da6;

         sa6.sin6_addr = spt->slirp->vhost_addr6;

-        sa6.sin6_port = recv_tp->udp.uh_dport;

+        sa6.sin6_port = hdr->udp.uh_dport;

         da6.sin6_addr = ((struct sockaddr_in6 *)&spt->client_addr)->sin6_addr;

         da6.sin6_port = spt->client_port;

@@ -163,7 +163,7 @@ static void tftp_udp_output(struct tftp_session *spt, struct mbuf *m,

         struct sockaddr_in sa4, da4;

         sa4.sin_addr = spt->slirp->vhost_addr;

-        sa4.sin_port = recv_tp->udp.uh_dport;

+        sa4.sin_port = hdr->udp.uh_dport;

         da4.sin_addr = ((struct sockaddr_in *)&spt->client_addr)->sin_addr;

         da4.sin_port = spt->client_port;

@@ -185,14 +185,14 @@ static int tftp_send_oack(struct tftp_session *spt, const char *keys[],

     tp = tftp_prep_mbuf_data(spt, m);

-    tp->tp_op = htons(TFTP_OACK);

+    tp->hdr.tp_op = htons(TFTP_OACK);

     for (i = 0; i < nb; i++) {

         n += slirp_fmt0(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%s", keys[i]);

         n += slirp_fmt0(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%u", values[i]);

     }

-    m->m_len = G_SIZEOF_MEMBER(struct tftp_t, tp_op) + n;

-    tftp_udp_output(spt, m, recv_tp);

+    m->m_len = G_SIZEOF_MEMBER(struct tftp_t, hdr.tp_op) + n;

+    tftp_udp_output(spt, m, &recv_tp->hdr);

     return 0;

 }

@@ -213,21 +213,21 @@ static void tftp_send_error(struct tftp_session *spt, uint16_t errorcode,

     tp = tftp_prep_mbuf_data(spt, m);

-    tp->tp_op = htons(TFTP_ERROR);

+    tp->hdr.tp_op = htons(TFTP_ERROR);

     tp->x.tp_error.tp_error_code = htons(errorcode);

     slirp_pstrcpy((char *)tp->x.tp_error.tp_msg, sizeof(tp->x.tp_error.tp_msg),

                   msg);

     m->m_len = sizeof(struct tftp_t) - (TFTP_BLOCKSIZE_MAX + 2) + 3 +

                strlen(msg) - sizeof(struct udphdr);

-    tftp_udp_output(spt, m, recv_tp);

+    tftp_udp_output(spt, m, &recv_tp->hdr);

 out:

     tftp_session_terminate(spt);

 }

 static void tftp_send_next_block(struct tftp_session *spt,

-                                 struct tftp_t *recv_tp)

+                                 struct tftphdr *hdr)

 {

     struct mbuf *m;

     struct tftp_t *tp;

@@ -241,7 +241,7 @@ static void tftp_send_next_block(struct tftp_session *spt,

     tp = tftp_prep_mbuf_data(spt, m);

-    tp->tp_op = htons(TFTP_DATA);

+    tp->hdr.tp_op = htons(TFTP_DATA);

     tp->x.tp_data.tp_block_nr = htons((spt->block_nr + 1) & 0xffff);

     nobytes = tftp_read_data(spt, spt->block_nr, tp->x.tp_data.tp_buf,

@@ -259,7 +259,7 @@ static void tftp_send_next_block(struct tftp_session *spt,

     m->m_len = sizeof(struct tftp_t) - (TFTP_BLOCKSIZE_MAX - nobytes) -

                sizeof(struct udphdr);

-    tftp_udp_output(spt, m, recv_tp);

+    tftp_udp_output(spt, m, hdr);

     if (nobytes == spt->block_size) {

         tftp_session_update(spt);

@@ -282,12 +282,12 @@ static void tftp_handle_rrq(Slirp *slirp, struct sockaddr_storage *srcsas,

     int nb_options = 0;

     /* check if a session already exists and if so terminate it */

-    s = tftp_session_find(slirp, srcsas, tp);

+    s = tftp_session_find(slirp, srcsas, &tp->hdr);

     if (s >= 0) {

         tftp_session_terminate(&slirp->tftp_sessions[s]);

     }

-    s = tftp_session_allocate(slirp, srcsas, tp);

+    s = tftp_session_allocate(slirp, srcsas, &tp->hdr);

     if (s < 0) {

         return;

@@ -413,29 +413,29 @@ static void tftp_handle_rrq(Slirp *slirp, struct sockaddr_storage *srcsas,

     }

     spt->block_nr = 0;

-    tftp_send_next_block(spt, tp);

+    tftp_send_next_block(spt, &tp->hdr);

 }

 static void tftp_handle_ack(Slirp *slirp, struct sockaddr_storage *srcsas,

-                            struct tftp_t *tp, int pktlen)

+                            struct tftphdr *hdr)

 {

     int s;

-    s = tftp_session_find(slirp, srcsas, tp);

+    s = tftp_session_find(slirp, srcsas, hdr);

     if (s < 0) {

         return;

     }

-    tftp_send_next_block(&slirp->tftp_sessions[s], tp);

+    tftp_send_next_block(&slirp->tftp_sessions[s], hdr);

 }

 static void tftp_handle_error(Slirp *slirp, struct sockaddr_storage *srcsas,

-                              struct tftp_t *tp, int pktlen)

+                              struct tftphdr *hdr)

 {

     int s;

-    s = tftp_session_find(slirp, srcsas, tp);

+    s = tftp_session_find(slirp, srcsas, hdr);

     if (s < 0) {

         return;

@@ -446,23 +446,25 @@ static void tftp_handle_error(Slirp *slirp, struct sockaddr_storage *srcsas,

 void tftp_input(struct sockaddr_storage *srcsas, struct mbuf *m)

 {

-    struct tftp_t *tp = mtod_check(m, offsetof(struct tftp_t, x.tp_buf));

+    struct tftphdr *hdr = mtod_check(m, sizeof(struct tftphdr));

-    if (tp == NULL) {

+    if (hdr == NULL) {

         return;

     }

-    switch (ntohs(tp->tp_op)) {

+    switch (ntohs(hdr->tp_op)) {

     case TFTP_RRQ:

-        tftp_handle_rrq(m->slirp, srcsas, tp, m->m_len);

+        tftp_handle_rrq(m->slirp, srcsas,

+                        mtod(m, struct tftp_t *),

+                        m->m_len);

         break;

     case TFTP_ACK:

-        tftp_handle_ack(m->slirp, srcsas, tp, m->m_len);

+        tftp_handle_ack(m->slirp, srcsas, hdr);

         break;

     case TFTP_ERROR:

-        tftp_handle_error(m->slirp, srcsas, tp, m->m_len);

+        tftp_handle_error(m->slirp, srcsas, hdr);

         break;

     }

 }

diff --git a/src/tftp.h b/src/tftp.h

index 6d75478..cafab03 100644

--- a/src/tftp.h

+++ b/src/tftp.h

@@ -20,9 +20,13 @@

 #define TFTP_FILENAME_MAX 512

 #define TFTP_BLOCKSIZE_MAX 1428

-struct tftp_t {

+struct tftphdr {

     struct udphdr udp;

     uint16_t tp_op;

+} SLIRP_PACKED;

+

+struct tftp_t {

+    struct tftphdr hdr;

     union {

         struct {

             uint16_t tp_block_nr;

-- 

2.29.0