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 --- 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 --- 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