From: Guus Sliepen Date: Mon, 1 Mar 2010 22:35:02 +0000 (+0100) Subject: Simplify reading lines from configuration files. X-Git-Tag: release-1.0.13~19 X-Git-Url: https://tinc-vpn.org/git/browse?a=commitdiff_plain;h=21f33b638291c2ffe7156e6c1e0df339f855d831;p=tinc Simplify reading lines from configuration files. Instead of allocating storage for each line read, we now read into fixed-size buffers on the stack. This fixes a case where a malformed configuration file could crash tinc. --- diff --git a/src/conf.c b/src/conf.c index e67c7ac1..f64fb221 100644 --- a/src/conf.c +++ b/src/conf.c @@ -26,6 +26,7 @@ #include "conf.h" #include "logger.h" #include "netutl.h" /* for str2address */ +#include "protocol.h" #include "utils.h" /* for cp */ #include "xalloc.h" @@ -206,111 +207,60 @@ bool get_config_subnet(const config_t *cfg, subnet_t ** result) { } /* - Read exactly one line and strip the trailing newline if any. If the - file was on EOF, return NULL. Otherwise, return all the data in a - dynamically allocated buffer. - - If line is non-NULL, it will be used as an initial buffer, to avoid - unnecessary mallocing each time this function is called. If buf is - given, and buf needs to be expanded, the var pointed to by buflen - will be increased. + Read exactly one line and strip the trailing newline if any. */ -static char *readline(FILE * fp, char **buf, size_t *buflen) { +static char *readline(FILE * fp, char *buf, size_t buflen) { char *newline = NULL; char *p; - char *line; /* The array that contains everything that has been read so far */ - char *idx; /* Read into this pointer, which points to an offset within line */ - size_t size, newsize; /* The size of the current array pointed to by line */ - size_t maxlen; /* Maximum number of characters that may be read with fgets. This is newsize - oldsize. */ if(feof(fp)) return NULL; - if(buf && buflen) { - size = *buflen; - line = *buf; - } else { - size = 100; - line = xmalloc(size); - } - - maxlen = size; - idx = line; - *idx = 0; - - for(;;) { - errno = 0; - p = fgets(idx, maxlen, fp); + p = fgets(buf, buflen, fp); - if(!p) { /* EOF or error */ - if(feof(fp)) - break; + if(!p) + return NULL; - /* otherwise: error; let the calling function print an error message if applicable */ - free(line); - return NULL; - } + newline = strchr(p, '\n'); - newline = strchr(p, '\n'); - - if(!newline) { /* We haven't yet read everything to the end of the line */ - newsize = size << 1; - line = xrealloc(line, newsize); - idx = &line[size - 1]; - maxlen = newsize - size + 1; - size = newsize; - } else { - *newline = '\0'; /* kill newline */ - if(newline > p && newline[-1] == '\r') /* and carriage return if necessary */ - newline[-1] = '\0'; - break; /* yay */ - } - } + if(!newline) + return NULL; - if(buf && buflen) { - *buflen = size; - *buf = line; - } + *newline = '\0'; /* kill newline */ + if(newline > p && newline[-1] == '\r') /* and carriage return if necessary */ + newline[-1] = '\0'; - return line; + return buf; } /* Parse a configuration file and put the results in the configuration tree starting at *base. */ -int read_config_file(avl_tree_t *config_tree, const char *fname) { - int err = -2; /* Parse error */ +bool read_config_file(avl_tree_t *config_tree, const char *fname) { FILE *fp; - char *buffer, *line; + char buffer[MAX_STRING_SIZE]; + char *line; char *variable, *value, *eol; int lineno = 0; int len; bool ignore = false; config_t *cfg; - size_t bufsize; + bool result = false; fp = fopen(fname, "r"); if(!fp) { - logger(LOG_ERR, "Cannot open config file %s: %s", fname, - strerror(errno)); - return -3; + logger(LOG_ERR, "Cannot open config file %s: %s", fname, strerror(errno)); + return false; } - bufsize = 100; - buffer = xmalloc(bufsize); - for(;;) { - if(feof(fp)) { - err = 0; - break; - } - - line = readline(fp, &buffer, &bufsize); + line = readline(fp, buffer, sizeof buffer); if(!line) { - err = -1; + if(feof(fp)) + result = true; break; } @@ -361,46 +311,46 @@ int read_config_file(avl_tree_t *config_tree, const char *fname) { config_add(config_tree, cfg); } - free(buffer); fclose(fp); - return err; + return result; } bool read_server_config() { char *fname; - int x; + bool x; xasprintf(&fname, "%s/tinc.conf", confbase); x = read_config_file(config_tree, fname); - if(x == -1) { /* System error: complain */ + if(!x) { /* System error: complain */ logger(LOG_ERR, "Failed to read `%s': %s", fname, strerror(errno)); } free(fname); - return x == 0; + return x; } FILE *ask_and_open(const char *filename, const char *what) { FILE *r; char *directory; - char *fn; + char line[PATH_MAX]; + const char *fn; /* Check stdin and stdout */ if(!isatty(0) || !isatty(1)) { /* Argh, they are running us from a script or something. Write the files to the current directory and let them burn in hell for ever. */ - fn = xstrdup(filename); + fn = filename; } else { /* Ask for a file and/or directory name. */ fprintf(stdout, "Please enter a file to save %s to [%s]: ", what, filename); fflush(stdout); - fn = readline(stdin, NULL, NULL); + fn = readline(stdin, line, sizeof line); if(!fn) { fprintf(stderr, "Error while reading stdin: %s\n", @@ -410,7 +360,7 @@ FILE *ask_and_open(const char *filename, const char *what) { if(!strlen(fn)) /* User just pressed enter. */ - fn = xstrdup(filename); + fn = filename; } #ifdef HAVE_MINGW @@ -423,7 +373,6 @@ FILE *ask_and_open(const char *filename, const char *what) { directory = get_current_dir_name(); xasprintf(&p, "%s/%s", directory, fn); - free(fn); free(directory); fn = p; } @@ -437,12 +386,9 @@ FILE *ask_and_open(const char *filename, const char *what) { if(!r) { fprintf(stderr, "Error opening file `%s': %s\n", fn, strerror(errno)); - free(fn); return NULL; } - free(fn); - return r; } diff --git a/src/conf.h b/src/conf.h index be497336..dae4eab8 100644 --- a/src/conf.h +++ b/src/conf.h @@ -54,7 +54,7 @@ extern bool get_config_string(const config_t *, char **); extern bool get_config_address(const config_t *, struct addrinfo **); extern bool get_config_subnet(const config_t *, struct subnet_t **); -extern int read_config_file(avl_tree_t *, const char *); +extern bool read_config_file(avl_tree_t *, const char *); extern bool read_server_config(void); extern FILE *ask_and_open(const char *, const char *); extern bool is_safe_path(const char *);