Skip to content

Commit

Permalink
curl: use curlx_dynbuf for realloc when loading config files
Browse files Browse the repository at this point in the history
... fixes an integer overflow at the same time.

Reported-by: ihsinme on github
Assisted-by: Jay Satiro

Closes curl#5946
  • Loading branch information
bagder committed Sep 14, 2020
1 parent c4ea71a commit 47dd957
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 42 deletions.
2 changes: 2 additions & 0 deletions projects/generate.bat
Expand Up @@ -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
Expand All @@ -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" (
Expand Down
6 changes: 4 additions & 2 deletions src/Makefile.inc
Expand Up @@ -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 \
Expand All @@ -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 \
Expand Down
67 changes: 31 additions & 36 deletions src/tool_parsecfg.c
Expand Up @@ -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 */

Expand All @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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 */
}
4 changes: 2 additions & 2 deletions tests/data/test558
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion tests/libtest/Makefile.inc
Expand Up @@ -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@
Expand Down
5 changes: 4 additions & 1 deletion winbuild/MakefileBuild.vc
Expand Up @@ -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)
Expand All @@ -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)

Expand Down

0 comments on commit 47dd957

Please sign in to comment.