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

diff --git a/projects/generate.bat b/projects/generate.bat
index 88979683f58992fdcd2892c8e5f1fc139a207f45..bc50245e8cbfd33c0309dfbcffa1a16d29619cf4 100644 (file)
--- 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 c0b9ad8645dcf07fa1663419d34b22db9e795a30..6f236fecc3ca583f09d05e8dc5effd48311b4e68 100644 (file)
--- 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 4e56492cb390a10b74dac2764746ee46f7382beb..f09180ed4cf4962f76480feee55c68a026c63974 100644 (file)
--- 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 d5aa0e087fb7fe430d7ff245df9e2546a324e134..f313e813acc993594fb12ce77815eb1d16cc9d4f 100644 (file)
--- a/tests/data/test558
+++ b/tests/data/test558
@@ -38,8 +38,8 @@ nothing
 <file name="log/memdump">
 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()
 </file>
 <stripfile>

diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc
index 88dd6852902c6cd579b9317ea790a4bbe4977be2..4904515cd1afce046da496abb3bee6429125d84d 100644 (file)
--- 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 eefc091a25a906ee74fbbc7f16d8d607fa95dc3c..6460e18f2a1c5fc50fa56982d61f733a36371f7c 100644 (file)
--- a/winbuild/MakefileBuild.vc
+++ b/winbuild/MakefileBuild.vc
@@ -620,7 +620,8 @@ CURL_FROM_LIBCURL=$(CURL_DIROBJ)\tool_hugehelp.obj \
  $(CURL_DIROBJ)\warnless.obj \\r
  $(CURL_DIROBJ)\curl_ctype.obj \\r
  $(CURL_DIROBJ)\curl_multibyte.obj \\r
- $(CURL_DIROBJ)\version_win32.obj\r
+ $(CURL_DIROBJ)\version_win32.obj \\r
+ $(CURL_DIROBJ)\dynbuf.obj\r
  \r
 $(PROGRAM_NAME): $(CURL_DIROBJ) $(CURL_FROM_LIBCURL) $(EXE_OBJS)\r
        $(CURL_LINK) $(CURL_LFLAGS) $(CURL_LIBCURL_LIBNAME) $(WIN_LIBS) $(CURL_FROM_LIBCURL) $(EXE_OBJS)\r
@@ -643,6 +644,8 @@ $(CURL_DIROBJ)\curl_multibyte.obj: ../lib/curl_multibyte.c
        $(CURL_CC) $(CURL_CFLAGS) /Fo"$@" ../lib/curl_multibyte.c\r
 $(CURL_DIROBJ)\version_win32.obj: ../lib/version_win32.c\r
        $(CURL_CC) $(CURL_CFLAGS) /Fo"$@" ../lib/version_win32.c\r
+$(CURL_DIROBJ)\dynbuf.obj: ../lib/dynbuf.c\r
+       $(CURL_CC) $(CURL_CFLAGS) /Fo"$@" ../lib/dynbuf.c\r
 $(CURL_DIROBJ)\curl.res: $(CURL_SRC_DIR)\curl.rc\r
        rc $(CURL_RC_FLAGS)\r
 \r