From 22b3d1cf0216f4369f01678c587da265c2e465af Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Sat, 28 Nov 2020 00:27:21 +0100 Subject: [PATCH] ftp: make wc_statemach loop instead of recurse CVE-2020-8285 Fixes #6255 Bug: https://curl.se/docs/CVE-2020-8285.html Reported-by: xnynx on github Upstream-commit: 69a358f2186e04cf44698b5100332cbf1ee7f01d Signed-off-by: Kamil Dudka --- lib/ftp.c | 204 +++++++++++++++++++++++++++--------------------------- 1 file changed, 103 insertions(+), 101 deletions(-) diff --git a/lib/ftp.c b/lib/ftp.c index 7dbf080..482ab3a 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -3786,130 +3786,132 @@ static CURLcode init_wc_data(struct connectdata *conn) return result; } -/* This is called recursively */ static CURLcode wc_statemach(struct connectdata *conn) { struct WildcardData * const wildcard = &(conn->data->wildcard); CURLcode result = CURLE_OK; - switch(wildcard->state) { - case CURLWC_INIT: - result = init_wc_data(conn); - if(wildcard->state == CURLWC_CLEAN) - /* only listing! */ - break; - wildcard->state = result ? CURLWC_ERROR : CURLWC_MATCHING; - break; + for(;;) { + switch(wildcard->state) { + case CURLWC_INIT: + result = init_wc_data(conn); + if(wildcard->state == CURLWC_CLEAN) + /* only listing! */ + return result; + wildcard->state = result ? CURLWC_ERROR : CURLWC_MATCHING; + return result; - case CURLWC_MATCHING: { - /* In this state is LIST response successfully parsed, so lets restore - previous WRITEFUNCTION callback and WRITEDATA pointer */ - struct ftp_wc *ftpwc = wildcard->protdata; - conn->data->set.fwrite_func = ftpwc->backup.write_function; - conn->data->set.out = ftpwc->backup.file_descriptor; - ftpwc->backup.write_function = ZERO_NULL; - ftpwc->backup.file_descriptor = NULL; - wildcard->state = CURLWC_DOWNLOADING; - - if(Curl_ftp_parselist_geterror(ftpwc->parser)) { - /* error found in LIST parsing */ - wildcard->state = CURLWC_CLEAN; - return wc_statemach(conn); - } - if(wildcard->filelist.size == 0) { - /* no corresponding file */ - wildcard->state = CURLWC_CLEAN; - return CURLE_REMOTE_FILE_NOT_FOUND; + case CURLWC_MATCHING: { + /* In this state is LIST response successfully parsed, so lets restore + previous WRITEFUNCTION callback and WRITEDATA pointer */ + struct ftp_wc *ftpwc = wildcard->protdata; + conn->data->set.fwrite_func = ftpwc->backup.write_function; + conn->data->set.out = ftpwc->backup.file_descriptor; + ftpwc->backup.write_function = ZERO_NULL; + ftpwc->backup.file_descriptor = NULL; + wildcard->state = CURLWC_DOWNLOADING; + + if(Curl_ftp_parselist_geterror(ftpwc->parser)) { + /* error found in LIST parsing */ + wildcard->state = CURLWC_CLEAN; + continue; + } + if(wildcard->filelist.size == 0) { + /* no corresponding file */ + wildcard->state = CURLWC_CLEAN; + return CURLE_REMOTE_FILE_NOT_FOUND; + } + continue; } - return wc_statemach(conn); - } - case CURLWC_DOWNLOADING: { - /* filelist has at least one file, lets get first one */ - struct ftp_conn *ftpc = &conn->proto.ftpc; - struct curl_fileinfo *finfo = wildcard->filelist.head->ptr; + case CURLWC_DOWNLOADING: { + /* filelist has at least one file, lets get first one */ + struct ftp_conn *ftpc = &conn->proto.ftpc; + struct curl_fileinfo *finfo = wildcard->filelist.head->ptr; - char *tmp_path = aprintf("%s%s", wildcard->path, finfo->filename); - if(!tmp_path) - return CURLE_OUT_OF_MEMORY; + char *tmp_path = aprintf("%s%s", wildcard->path, finfo->filename); + if(!tmp_path) + return CURLE_OUT_OF_MEMORY; - /* switch default "state.pathbuffer" and tmp_path, good to see - ftp_parse_url_path function to understand this trick */ - Curl_safefree(conn->data->state.pathbuffer); - conn->data->state.pathbuffer = tmp_path; - conn->data->state.path = tmp_path; - - infof(conn->data, "Wildcard - START of \"%s\"\n", finfo->filename); - if(conn->data->set.chunk_bgn) { - long userresponse; - Curl_set_in_callback(conn->data, true); - userresponse = conn->data->set.chunk_bgn( - finfo, wildcard->customptr, (int)wildcard->filelist.size); - Curl_set_in_callback(conn->data, false); - switch(userresponse) { - case CURL_CHUNK_BGN_FUNC_SKIP: - infof(conn->data, "Wildcard - \"%s\" skipped by user\n", - finfo->filename); - wildcard->state = CURLWC_SKIP; - return wc_statemach(conn); - case CURL_CHUNK_BGN_FUNC_FAIL: - return CURLE_CHUNK_FAILED; + /* switch default "state.pathbuffer" and tmp_path, good to see + ftp_parse_url_path function to understand this trick */ + Curl_safefree(conn->data->state.pathbuffer); + conn->data->state.pathbuffer = tmp_path; + conn->data->state.path = tmp_path; + + infof(conn->data, "Wildcard - START of \"%s\"\n", finfo->filename); + if(conn->data->set.chunk_bgn) { + long userresponse; + Curl_set_in_callback(conn->data, true); + userresponse = conn->data->set.chunk_bgn( + finfo, wildcard->customptr, (int)wildcard->filelist.size); + Curl_set_in_callback(conn->data, false); + switch(userresponse) { + case CURL_CHUNK_BGN_FUNC_SKIP: + infof(conn->data, "Wildcard - \"%s\" skipped by user\n", + finfo->filename); + wildcard->state = CURLWC_SKIP; + continue; + case CURL_CHUNK_BGN_FUNC_FAIL: + return CURLE_CHUNK_FAILED; + } } - } - if(finfo->filetype != CURLFILETYPE_FILE) { - wildcard->state = CURLWC_SKIP; - return wc_statemach(conn); - } + if(finfo->filetype != CURLFILETYPE_FILE) { + wildcard->state = CURLWC_SKIP; + continue; + } - if(finfo->flags & CURLFINFOFLAG_KNOWN_SIZE) - ftpc->known_filesize = finfo->size; + if(finfo->flags & CURLFINFOFLAG_KNOWN_SIZE) + ftpc->known_filesize = finfo->size; - result = ftp_parse_url_path(conn); - if(result) - return result; + result = ftp_parse_url_path(conn); + if(result) + return result; - /* we don't need the Curl_fileinfo of first file anymore */ - Curl_llist_remove(&wildcard->filelist, wildcard->filelist.head, NULL); + /* we don't need the Curl_fileinfo of first file anymore */ + Curl_llist_remove(&wildcard->filelist, wildcard->filelist.head, NULL); - if(wildcard->filelist.size == 0) { /* remains only one file to down. */ - wildcard->state = CURLWC_CLEAN; - /* after that will be ftp_do called once again and no transfer - will be done because of CURLWC_CLEAN state */ - return CURLE_OK; + if(wildcard->filelist.size == 0) { /* remains only one file to down. */ + wildcard->state = CURLWC_CLEAN; + /* after that will be ftp_do called once again and no transfer + will be done because of CURLWC_CLEAN state */ + return CURLE_OK; + } + return result; } - } break; - case CURLWC_SKIP: { - if(conn->data->set.chunk_end) { - Curl_set_in_callback(conn->data, true); - conn->data->set.chunk_end(conn->data->wildcard.customptr); - Curl_set_in_callback(conn->data, false); + case CURLWC_SKIP: { + if(conn->data->set.chunk_end) { + Curl_set_in_callback(conn->data, true); + conn->data->set.chunk_end(conn->data->wildcard.customptr); + Curl_set_in_callback(conn->data, false); + } + Curl_llist_remove(&wildcard->filelist, wildcard->filelist.head, NULL); + wildcard->state = (wildcard->filelist.size == 0) ? + CURLWC_CLEAN : CURLWC_DOWNLOADING; + continue; } - Curl_llist_remove(&wildcard->filelist, wildcard->filelist.head, NULL); - wildcard->state = (wildcard->filelist.size == 0) ? - CURLWC_CLEAN : CURLWC_DOWNLOADING; - return wc_statemach(conn); - } - case CURLWC_CLEAN: { - struct ftp_wc *ftpwc = wildcard->protdata; - result = CURLE_OK; - if(ftpwc) - result = Curl_ftp_parselist_geterror(ftpwc->parser); + case CURLWC_CLEAN: { + struct ftp_wc *ftpwc = wildcard->protdata; + result = CURLE_OK; + if(ftpwc) + result = Curl_ftp_parselist_geterror(ftpwc->parser); - wildcard->state = result ? CURLWC_ERROR : CURLWC_DONE; - } break; + wildcard->state = result ? CURLWC_ERROR : CURLWC_DONE; + return result; + } - case CURLWC_DONE: - case CURLWC_ERROR: - case CURLWC_CLEAR: - if(wildcard->dtor) - wildcard->dtor(wildcard->protdata); - break; + case CURLWC_DONE: + case CURLWC_ERROR: + case CURLWC_CLEAR: + if(wildcard->dtor) + wildcard->dtor(wildcard->protdata); + return result; + } } - - return result; + /* UNREACHABLE */ } /*********************************************************************** -- 2.26.2