From fe9b4ce23f82a3b2564123bbc3c674cc07a04f2b Mon Sep 17 00:00:00 2001

From: Thomas Vegas <>

Date: Sat, 31 Aug 2019 16:59:56 +0200

Subject: [PATCH 1/2] tftp: return error when packet is too small for options

Upstream-commit: 82f3ba3806a34fe94dcf9e5c9b88deda6679ca1b

Signed-off-by: Kamil Dudka <kdudka@redhat.com>

---

 lib/tftp.c | 76 +++++++++++++++++++++++++++++++++---------------------

 1 file changed, 46 insertions(+), 30 deletions(-)

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

index 022648a..8ab9d71 100644

--- a/lib/tftp.c

+++ b/lib/tftp.c

@@ -405,13 +405,14 @@ static CURLcode tftp_parse_option_ack(tftp_state_data_t *state,

   return CURLE_OK;

 }

-static size_t tftp_option_add(tftp_state_data_t *state, size_t csize,

-                              char *buf, const char *option)

+static CURLcode tftp_option_add(tftp_state_data_t *state, size_t *csize,

+                                char *buf, const char *option)

 {

-  if(( strlen(option) + csize + 1 ) > (size_t)state->blksize)

-    return 0;

+  if(( strlen(option) + *csize + 1) > (size_t)state->blksize)

+    return CURLE_TFTP_ILLEGAL;

   strcpy(buf, option);

-  return( strlen(option) + 1 );

+  *csize += strlen(option) + 1;

+  return CURLE_OK;

 }

 static CURLcode tftp_connect_for_tx(tftp_state_data_t *state,

@@ -498,31 +499,46 @@ static CURLcode tftp_send_first(tftp_state_data_t *state, tftp_event_t event)

     sbytes = 4 + strlen(filename) + strlen(mode);

     /* add tsize option */

-    if(data->set.upload && (data->set.infilesize != -1))

-      snprintf( buf, sizeof(buf), "%" FORMAT_OFF_T, data->set.infilesize );

-    else

-      strcpy(buf, "0"); /* the destination is large enough */

-

-    sbytes += tftp_option_add(state, sbytes,

-                              (char *)state->spacket.data+sbytes,

-                              TFTP_OPTION_TSIZE);

-    sbytes += tftp_option_add(state, sbytes,

-                              (char *)state->spacket.data+sbytes, buf);

-    /* add blksize option */

-    snprintf( buf, sizeof(buf), "%d", state->requested_blksize );

-    sbytes += tftp_option_add(state, sbytes,

-                              (char *)state->spacket.data+sbytes,

-                              TFTP_OPTION_BLKSIZE);

-    sbytes += tftp_option_add(state, sbytes,

-                              (char *)state->spacket.data+sbytes, buf );

-

-    /* add timeout option */

-    snprintf( buf, sizeof(buf), "%d", state->retry_time);

-    sbytes += tftp_option_add(state, sbytes,

-                              (char *)state->spacket.data+sbytes,

-                              TFTP_OPTION_INTERVAL);

-    sbytes += tftp_option_add(state, sbytes,

-                              (char *)state->spacket.data+sbytes, buf );

+    {

+      CURLcode result;

+      if(data->set.upload && (data->set.infilesize != -1))

+        snprintf( buf, sizeof(buf), "%" FORMAT_OFF_T, data->set.infilesize );

+      else

+        strcpy(buf, "0"); /* the destination is large enough */

+

+      result = tftp_option_add(state, &sbytes,

+                               (char *)state->spacket.data + sbytes,

+                               TFTP_OPTION_TSIZE);

+      if(result == CURLE_OK)

+        result = tftp_option_add(state, &sbytes,

+                                 (char *)state->spacket.data + sbytes, buf);

+

+      /* add blksize option */

+      snprintf(buf, sizeof(buf), "%d", state->requested_blksize);

+      if(result == CURLE_OK)

+        result = tftp_option_add(state, &sbytes,

+                                 (char *)state->spacket.data + sbytes,

+                                 TFTP_OPTION_BLKSIZE);

+      if(result == CURLE_OK)

+        result = tftp_option_add(state, &sbytes,

+                                 (char *)state->spacket.data + sbytes, buf);

+

+      /* add timeout option */

+      snprintf(buf, sizeof(buf), "%d", state->retry_time);

+      if(result == CURLE_OK)

+        result = tftp_option_add(state, &sbytes,

+                                 (char *)state->spacket.data + sbytes,

+                                 TFTP_OPTION_INTERVAL);

+      if(result == CURLE_OK)

+        result = tftp_option_add(state, &sbytes,

+                                 (char *)state->spacket.data + sbytes, buf);

+

+      if(result != CURLE_OK) {

+        failf(data, "TFTP buffer too small for options");

+        free(filename);

+        return CURLE_TFTP_ILLEGAL;

+      }

+    }

     /* the typecase for the 3rd argument is mostly for systems that do

        not have a size_t argument, like older unixes that want an 'int' */

-- 

2.20.1

From ea5ca5f9ee1a4588061fc8b5a10c5b65fb68a0cd Mon Sep 17 00:00:00 2001

From: Thomas Vegas <>

Date: Sat, 31 Aug 2019 17:30:51 +0200

Subject: [PATCH 2/2] tftp: Alloc maximum blksize, and use default unless OACK

 is received

Fixes potential buffer overflow from 'recvfrom()', should the server

return an OACK without blksize.

Bug: https://curl.haxx.se/docs/CVE-2019-5482.html

CVE-2019-5482

Upstream-commit: facb0e4662415b5f28163e853dc6742ac5fafb3d

Signed-off-by: Kamil Dudka <kdudka@redhat.com>

---

 lib/tftp.c | 12 +++++++++---

 1 file changed, 9 insertions(+), 3 deletions(-)

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

index 8ab9d71..af3b288 100644

--- a/lib/tftp.c

+++ b/lib/tftp.c

@@ -961,6 +961,7 @@ static CURLcode tftp_connect(struct connectdata *conn, bool *done)

   CURLcode code;

   tftp_state_data_t *state;

   int blksize, rc;

+  int need_blksize;

   blksize = TFTP_BLKSIZE_DEFAULT;

@@ -979,15 +980,20 @@ static CURLcode tftp_connect(struct connectdata *conn, bool *done)

       return CURLE_TFTP_ILLEGAL;

   }

+  need_blksize = blksize;

+  /* default size is the fallback when no OACK is received */

+  if(need_blksize < TFTP_BLKSIZE_DEFAULT)

+    need_blksize = TFTP_BLKSIZE_DEFAULT;

+

   if(!state->rpacket.data) {

-    state->rpacket.data = calloc(1, blksize + 2 + 2);

+    state->rpacket.data = calloc(1, need_blksize + 2 + 2);

     if(!state->rpacket.data)

       return CURLE_OUT_OF_MEMORY;

   }

   if(!state->spacket.data) {

-    state->spacket.data = calloc(1, blksize + 2 + 2);

+    state->spacket.data = calloc(1, need_blksize + 2 + 2);

     if(!state->spacket.data)

       return CURLE_OUT_OF_MEMORY;

@@ -1001,7 +1007,7 @@ static CURLcode tftp_connect(struct connectdata *conn, bool *done)

   state->sockfd = state->conn->sock[FIRSTSOCKET];

   state->state = TFTP_STATE_START;

   state->error = TFTP_ERR_NONE;

-  state->blksize = blksize;

+  state->blksize = TFTP_BLKSIZE_DEFAULT; /* Unless updated by OACK response */

   state->requested_blksize = blksize;

   ((struct sockaddr *)&state->local_addr)->sa_family =

-- 

2.20.1