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