From 47dd957daff9199daa5fabfc557fe8c36d61f375 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 9 Sep 2020 15:41:25 +0200 Subject: [PATCH] curl: use curlx_dynbuf for realloc when loading config files ... fixes an integer overflow at the same time. Reported-by: ihsinme on github Assisted-by: Jay Satiro Closes #5946 --- projects/generate.bat | 2 ++ src/Makefile.inc | 6 ++-- src/tool_parsecfg.c | 67 ++++++++++++++++++-------------------- tests/data/test558 | 4 +-- tests/libtest/Makefile.inc | 2 +- winbuild/MakefileBuild.vc | 5 ++- 6 files changed, 44 insertions(+), 42 deletions(-) diff --git a/projects/generate.bat b/projects/generate.bat index 88979683f..bc50245e8 100644 --- a/projects/generate.bat +++ b/projects/generate.bat @@ -287,6 +287,7 @@ rem call :element %1 lib "curl_ctype.c" %3 call :element %1 lib "curl_multibyte.c" %3 call :element %1 lib "version_win32.c" %3 + call :element %1 lib "dynbuf.c" %3 ) else if "!var!" == "CURL_SRC_X_H_FILES" ( call :element %1 lib "config-win32.h" %3 call :element %1 lib "curl_setup.h" %3 @@ -296,6 +297,7 @@ rem call :element %1 lib "curl_ctype.h" %3 call :element %1 lib "curl_multibyte.h" %3 call :element %1 lib "version_win32.h" %3 + call :element %1 lib "dynbuf.h" %3 ) else if "!var!" == "CURL_LIB_C_FILES" ( for /f "delims=" %%c in ('dir /b ..\lib\*.c') do call :element %1 lib "%%c" %3 ) else if "!var!" == "CURL_LIB_H_FILES" ( diff --git a/src/Makefile.inc b/src/Makefile.inc index c0b9ad864..6f236fecc 100644 --- a/src/Makefile.inc +++ b/src/Makefile.inc @@ -36,7 +36,8 @@ CURLX_CFILES = \ ../lib/warnless.c \ ../lib/curl_ctype.c \ ../lib/curl_multibyte.c \ - ../lib/version_win32.c + ../lib/version_win32.c \ + ../lib/dynbuf.c CURLX_HFILES = \ ../lib/curl_setup.h \ @@ -45,7 +46,8 @@ CURLX_HFILES = \ ../lib/warnless.h \ ../lib/curl_ctype.h \ ../lib/curl_multibyte.h \ - ../lib/version_win32.h + ../lib/version_win32.h \ + ../lib/dynbuf.h CURL_CFILES = \ slist_wc.c \ diff --git a/src/tool_parsecfg.c b/src/tool_parsecfg.c index 4e56492cb..f09180ed4 100644 --- a/src/tool_parsecfg.c +++ b/src/tool_parsecfg.c @@ -31,6 +31,7 @@ #include "tool_homedir.h" #include "tool_msgs.h" #include "tool_parsecfg.h" +#include "dynbuf.h" #include "memdebug.h" /* keep this as LAST include */ @@ -39,7 +40,9 @@ #define ISSEP(x,dash) (!dash && (((x) == '=') || ((x) == ':'))) static const char *unslashquote(const char *line, char *param); -static char *my_get_line(FILE *fp); + +#define MAX_CONFIG_LINE_LENGTH (100*1024) +static bool my_get_line(FILE *fp, struct curlx_dynbuf *, bool *error); #ifdef WIN32 static FILE *execpath(const char *filename) @@ -135,17 +138,23 @@ int parseconfig(const char *filename, struct GlobalConfig *global) if(file) { char *line; - char *aline; char *option; char *param; int lineno = 0; bool dashed_option; + struct curlx_dynbuf buf; + bool fileerror; + curlx_dyn_init(&buf, MAX_CONFIG_LINE_LENGTH); - while(NULL != (aline = my_get_line(file))) { + while(my_get_line(file, &buf, &fileerror)) { int res; bool alloced_param = FALSE; lineno++; - line = aline; + line = curlx_dyn_ptr(&buf); + if(!line) { + rc = 1; /* out of memory */ + break; + } /* line with # in the first non-blank column is a comment! */ while(*line && ISSPACE(*line)) @@ -158,7 +167,7 @@ int parseconfig(const char *filename, struct GlobalConfig *global) case '\n': case '*': case '\0': - Curl_safefree(aline); + curlx_dyn_reset(&buf); continue; } @@ -190,7 +199,6 @@ int parseconfig(const char *filename, struct GlobalConfig *global) param = malloc(strlen(line) + 1); /* parameter */ if(!param) { /* out of memory */ - Curl_safefree(aline); rc = 1; break; } @@ -280,10 +288,13 @@ int parseconfig(const char *filename, struct GlobalConfig *global) if(alloced_param) Curl_safefree(param); - Curl_safefree(aline); + curlx_dyn_reset(&buf); } + curlx_dyn_free(&buf); if(file != stdin) fclose(file); + if(fileerror) + rc = 1; } else rc = 1; /* couldn't open the file */ @@ -335,39 +346,23 @@ static const char *unslashquote(const char *line, char *param) /* * Reads a line from the given file, ensuring is NUL terminated. - * The pointer must be freed by the caller. - * NULL is returned on an out of memory condition. */ -static char *my_get_line(FILE *fp) +static bool my_get_line(FILE *fp, struct curlx_dynbuf *db, + bool *error) { char buf[4096]; - char *nl = NULL; - char *line = NULL; - + *error = FALSE; do { - if(NULL == fgets(buf, sizeof(buf), fp)) - break; - if(!line) { - line = strdup(buf); - if(!line) - return NULL; + /* fgets() returns s on success, and NULL on error or when end of file + occurs while no characters have been read. */ + if(!fgets(buf, sizeof(buf), fp)) + /* only if there's data in the line, return TRUE */ + return curlx_dyn_len(db) ? TRUE : FALSE; + if(curlx_dyn_add(db, buf)) { + *error = TRUE; /* error */ + return FALSE; /* stop reading */ } - else { - char *ptr; - size_t linelen = strlen(line); - ptr = realloc(line, linelen + strlen(buf) + 1); - if(!ptr) { - Curl_safefree(line); - return NULL; - } - line = ptr; - strcpy(&line[linelen], buf); - } - nl = strchr(line, '\n'); - } while(!nl); + } while(!strchr(buf, '\n')); - if(nl) - *nl = '\0'; - - return line; + return TRUE; /* continue */ } diff --git a/tests/data/test558 b/tests/data/test558 index d5aa0e087..f313e813a 100644 --- a/tests/data/test558 +++ b/tests/data/test558 @@ -38,8 +38,8 @@ nothing MEM lib558.c: malloc() MEM lib558.c: free() -MEM strdup.c: realloc() -MEM strdup.c: realloc() +MEM dynbuf.c: realloc() +MEM dynbuf.c: realloc() MEM escape.c: free() diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc index 88dd68529..4904515cd 100644 --- a/tests/libtest/Makefile.inc +++ b/tests/libtest/Makefile.inc @@ -65,7 +65,7 @@ chkdecimalpoint_SOURCES = chkdecimalpoint.c ../../lib/mprintf.c \ ../../lib/curl_ctype.c ../../lib/dynbuf.c ../../lib/strdup.c chkdecimalpoint_LDADD = chkdecimalpoint_CPPFLAGS = $(AM_CPPFLAGS) -DCURL_STATICLIB \ - -DCURLX_NO_MEMORY_CALLBACKS + -DCURLX_NO_MEMORY_CALLBACKS -DBUILDING_LIBCURL chkhostname_SOURCES = chkhostname.c ../../lib/curl_gethostname.c chkhostname_LDADD = @CURL_NETWORK_LIBS@ diff --git a/winbuild/MakefileBuild.vc b/winbuild/MakefileBuild.vc index eefc091a2..6460e18f2 100644 --- a/winbuild/MakefileBuild.vc +++ b/winbuild/MakefileBuild.vc @@ -620,7 +620,8 @@ CURL_FROM_LIBCURL=$(CURL_DIROBJ)\tool_hugehelp.obj \ $(CURL_DIROBJ)\warnless.obj \ $(CURL_DIROBJ)\curl_ctype.obj \ $(CURL_DIROBJ)\curl_multibyte.obj \ - $(CURL_DIROBJ)\version_win32.obj + $(CURL_DIROBJ)\version_win32.obj \ + $(CURL_DIROBJ)\dynbuf.obj $(PROGRAM_NAME): $(CURL_DIROBJ) $(CURL_FROM_LIBCURL) $(EXE_OBJS) $(CURL_LINK) $(CURL_LFLAGS) $(CURL_LIBCURL_LIBNAME) $(WIN_LIBS) $(CURL_FROM_LIBCURL) $(EXE_OBJS) @@ -643,6 +644,8 @@ $(CURL_DIROBJ)\curl_multibyte.obj: ../lib/curl_multibyte.c $(CURL_CC) $(CURL_CFLAGS) /Fo"$@" ../lib/curl_multibyte.c $(CURL_DIROBJ)\version_win32.obj: ../lib/version_win32.c $(CURL_CC) $(CURL_CFLAGS) /Fo"$@" ../lib/version_win32.c +$(CURL_DIROBJ)\dynbuf.obj: ../lib/dynbuf.c + $(CURL_CC) $(CURL_CFLAGS) /Fo"$@" ../lib/dynbuf.c $(CURL_DIROBJ)\curl.res: $(CURL_SRC_DIR)\curl.rc rc $(CURL_RC_FLAGS)