From 43d0f3fc76542d0859c9b84402c0483a22e02b68 Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Mon, 7 Jan 2008 12:29:30 +0000 Subject: [PATCH] rc_getline keeps expanding it's malloced buffer until it has read a whole line or EOF. All functions which read into static buffers have been changed to use fhis function to avoid any potential overflows and to ensure we really do read a long long config line. --- src/cc.mk | 16 +++++++-------- src/default.mk | 34 ------------------------------- src/librc/librc-daemon.c | 41 +++++++++++++++----------------------- src/librc/librc-depend.c | 43 ++++++++++++++++++++-------------------- src/librc/librc-misc.c | 42 ++++++++++++++++++++++++--------------- src/librc/librc.c | 43 +++++++++++----------------------------- src/librc/librc.h | 1 + src/librc/rc.h | 4 ++++ src/librc/rc.map | 1 + src/rc-misc.h | 3 --- src/rc/rc-logger.c | 10 +++++----- src/rc/rc-misc.c | 19 +++++++++--------- src/rc/rc-plugin.c | 6 +++--- src/rc/rc.c | 19 ++++++------------ src/rc/runscript.c | 4 ++-- 15 files changed, 114 insertions(+), 172 deletions(-) delete mode 100644 src/default.mk diff --git a/src/cc.mk b/src/cc.mk index d3c96a65..1dc1b3fe 100644 --- a/src/cc.mk +++ b/src/cc.mk @@ -6,7 +6,7 @@ CFLAGS ?= -O2 -pipe # GNU Make way of detecting gcc flags we can use check_gcc=$(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null >/dev/null 2>&1; \ - then echo "$(1)"; else echo "$(2)"; fi) + then echo "$(1)"; else echo "$(2)"; fi) # pmake check for extra cflags WEXTRA != for x in -Wdeclaration-after-statement -Wsequence-point -Wextra; do \ @@ -16,10 +16,10 @@ WEXTRA != for x in -Wdeclaration-after-statement -Wsequence-point -Wextra; do \ # Loads of nice flags to ensure our code is good CFLAGS += -pedantic -std=c99 \ - -Wall -Wunused -Wimplicit -Wshadow -Wformat=2 \ - -Wmissing-declarations -Wno-missing-prototypes -Wwrite-strings \ - -Wbad-function-cast -Wnested-externs -Wcomment -Winline \ - -Wchar-subscripts -Wcast-align -Wno-format-nonliteral \ - $(call check_gcc, -Wdeclaration-after-statement) \ - $(call check_gcc, -Wsequence-point) \ - $(call check_gcc, -Wextra) $(WEXTRA) + -Wall -Wunused -Wimplicit -Wshadow -Wformat=2 \ + -Wmissing-declarations -Wno-missing-prototypes -Wwrite-strings \ + -Wbad-function-cast -Wnested-externs -Wcomment -Winline \ + -Wchar-subscripts -Wcast-align -Wno-format-nonliteral \ + $(call check_gcc, -Wdeclaration-after-statement) \ + $(call check_gcc, -Wsequence-point) \ + $(call check_gcc, -Wextra) $(WEXTRA) diff --git a/src/default.mk b/src/default.mk deleted file mode 100644 index 10f4fcb2..00000000 --- a/src/default.mk +++ /dev/null @@ -1,34 +0,0 @@ -# Copyright 2007-2008 Roy Marples - -CC ?= gcc -AR ?= ar -RANLIB ?= ranlib -CFLAGS += -O2 -pipe -LDFLAGS += -L. -PICFLAG = -fPIC - -# GNU Make way of detecting gcc flags we can use -check_gcc=$(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null >/dev/null 2>&1; \ - then echo "$(1)"; else echo "$(2)"; fi) - -# pmake check for extra cflags -WEXTRA != for x in -Wdeclaration-after-statement -Wsequence-point -Wextra; do \ - if $(CC) $$x -S -o /dev/null -xc /dev/null >/dev/null 2>&1; \ - then echo -n "$$x "; fi \ - done - -# Loads of nice flags to ensure our code is good -CFLAGS += -pedantic -std=c99 \ - -Wall -Wunused -Wimplicit -Wshadow -Wformat=2 \ - -Wmissing-declarations -Wno-missing-prototypes -Wwrite-strings \ - -Wbad-function-cast -Wnested-externs -Wcomment -Winline \ - -Wchar-subscripts -Wcast-align -Wno-format-nonliteral \ - $(call check_gcc, -Wdeclaration-after-statement) \ - $(call check_gcc, -Wsequence-point) \ - $(call check_gcc, -Wextra) $(WEXTRA) - -# For debugging. -Werror is pointless due to ISO C issues with dlsym -#CFLAGS += -ggdb - -TOPDIR = .. -include $(TOPDIR)/default.mk diff --git a/src/librc/librc-daemon.c b/src/librc/librc-daemon.c index fe9509de..04852e0e 100644 --- a/src/librc/librc-daemon.c +++ b/src/librc/librc-daemon.c @@ -265,7 +265,7 @@ static bool _match_daemon (const char *path, const char *file, const char *mexec, const char *mname, const char *mpidfile) { - char *buffer; + char *line; char *ffile = rc_strcatpaths (path, file, (char *) NULL); FILE *fp; int lc = 0; @@ -281,19 +281,14 @@ static bool _match_daemon (const char *path, const char *file, if (! mpidfile) m += 100; - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); - memset (buffer, 0, RC_LINEBUFFER); - while ((fgets (buffer, RC_LINEBUFFER, fp))) { - int lb = strlen (buffer) - 1; - if (buffer[lb] == '\n') - buffer[lb] = 0; - - if (strcmp (buffer, mexec) == 0) + while ((line = rc_getline (fp))) { + if (strcmp (line, mexec) == 0) m += 1; - else if (mname && strcmp (buffer, mname) == 0) + else if (mname && strcmp (line, mname) == 0) m += 10; - else if (mpidfile && strcmp (buffer, mpidfile) == 0) + else if (mpidfile && strcmp (line, mpidfile) == 0) m += 100; + free (line); if (m == 111) break; @@ -302,7 +297,6 @@ static bool _match_daemon (const char *path, const char *file, if (lc > 5) break; } - free (buffer); fclose (fp); free (ffile); @@ -460,7 +454,7 @@ bool rc_service_daemons_crashed (const char *service) struct dirent *d; char *path; FILE *fp; - char *buffer = NULL; + char *line; char *exec = NULL; char *name = NULL; char *pidfile = NULL; @@ -481,9 +475,6 @@ bool rc_service_daemons_crashed (const char *service) return (false); } - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); - memset (buffer, 0, RC_LINEBUFFER); - while ((d = readdir (dp))) { if (d->d_name[0] == '.') continue; @@ -494,17 +485,17 @@ bool rc_service_daemons_crashed (const char *service) if (! fp) break; - while ((fgets (buffer, RC_LINEBUFFER, fp))) { - int lb = strlen (buffer) - 1; - if (buffer[lb] == '\n') - buffer[lb] = 0; - - p = buffer; - if ((token = strsep (&p, "=")) == NULL || ! p) + while ((line = rc_getline (fp))) { + p = line; + if ((token = strsep (&p, "=")) == NULL || ! p) { + free (line); continue; + } - if (strlen (p) == 0) + if (strlen (p) == 0) { + free (line); continue; + } if (strcmp (token, "exec") == 0) { if (exec) @@ -519,6 +510,7 @@ bool rc_service_daemons_crashed (const char *service) free (pidfile); pidfile = xstrdup (p); } + free (line); } fclose (fp); @@ -563,7 +555,6 @@ bool rc_service_daemons_crashed (const char *service) name = NULL; } - free (buffer); free (exec); free (name); free (dirpath); diff --git a/src/librc/librc-depend.c b/src/librc/librc-depend.c index 0ce95e60..aece3858 100644 --- a/src/librc/librc-depend.c +++ b/src/librc/librc-depend.c @@ -124,7 +124,7 @@ rc_depinfo_t *rc_deptree_load (void) rc_depinfo_t *deptree = NULL; rc_depinfo_t *depinfo = NULL; rc_deptype_t *deptype = NULL; - char buffer [RC_LINEBUFFER]; + char *line; char *type; char *p; char *e; @@ -133,26 +133,26 @@ rc_depinfo_t *rc_deptree_load (void) if (! (fp = fopen (RC_DEPTREE, "r"))) return (NULL); - while (fgets (buffer, RC_LINEBUFFER, fp)) + while ((line = rc_getline (fp))) { - p = buffer; + p = line; e = strsep (&p, "_"); if (! e || strcmp (e, "depinfo") != 0) - continue; + goto next; e = strsep (&p, "_"); if (! e || sscanf (e, "%d", &i) != 1) - continue; + goto next; if (! (type = strsep (&p, "_="))) - continue; + goto next; if (strcmp (type, "service") == 0) { /* Sanity */ e = get_shell_value (p); if (! e || strlen (e) == 0) - continue; + goto next; if (! deptree) { @@ -167,17 +167,17 @@ rc_depinfo_t *rc_deptree_load (void) memset (depinfo, 0, sizeof (rc_depinfo_t)); depinfo->service = xstrdup (e); deptype = NULL; - continue; + goto next; } e = strsep (&p, "="); if (! e || sscanf (e, "%d", &i) != 1) - continue; + goto next; /* Sanity */ e = get_shell_value (p); if (! e || strlen (e) == 0) - continue; + goto next; if (! deptype) { @@ -197,6 +197,9 @@ rc_depinfo_t *rc_deptree_load (void) deptype->type = xstrdup (type); rc_strlist_addsort (&deptype->services, e); + +next: + free (line); } fclose (fp); @@ -722,7 +725,7 @@ bool rc_deptree_update (void) rc_deptype_t *deptype = NULL; rc_deptype_t *dt; rc_deptype_t *last_deptype = NULL; - char *buffer = NULL; + char *line; int len; int i; int j; @@ -740,20 +743,14 @@ bool rc_deptree_update (void) deptree = xmalloc (sizeof (rc_depinfo_t)); memset (deptree, 0, sizeof (rc_depinfo_t)); - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); - memset (buffer, 0, RC_LINEBUFFER); /* Phase 2 */ - while (fgets (buffer, RC_LINEBUFFER, fp)) + while ((line = rc_getline (fp))) { - /* Trim the newline */ - if (buffer[strlen (buffer) - 1] == '\n') - buffer[strlen(buffer) -1] = 0; - - depends = buffer; + depends = line; service = strsep (&depends, " "); if (! service) - continue; + goto next; type = strsep (&depends, " "); for (depinfo = deptree; depinfo; depinfo = depinfo->next) @@ -778,7 +775,7 @@ bool rc_deptree_update (void) /* We may not have any depends */ if (! type || ! depends) - continue; + goto next; /* Get the type */ if (strcmp (type, "config") != 0) { @@ -844,9 +841,11 @@ bool rc_deptree_update (void) rc_strlist_delete (&dt->services, depend); } } + +next: + free (line); } pclose (fp); - free (buffer); /* Phase 3 - add our providors to the tree */ for (depinfo = deptree; depinfo; depinfo = depinfo->next) diff --git a/src/librc/librc-misc.c b/src/librc/librc-misc.c index b8f59936..8cbd6a6d 100644 --- a/src/librc/librc-misc.c +++ b/src/librc/librc-misc.c @@ -105,6 +105,30 @@ char *rc_strcatpaths (const char *path1, const char *paths, ...) } librc_hidden_def(rc_strcatpaths) +char *rc_getline (FILE *fp) +{ + char *line = NULL; + size_t len = 0; + size_t last = 0; + + if (feof (fp)) + return (NULL); + + do { + len += BUFSIZ; + line = xrealloc (line, sizeof (char) * len); + fgets (line + last, BUFSIZ, fp); + last = strlen (line + last) - 1; + } while (! feof (fp) && line[last] != '\n'); + + /* Trim the trailing newline */ + if (line[last] == '\n') + line[last] = '\0'; + + return (line); +} +librc_hidden_def(rc_getline) + char **rc_config_list (const char *file) { FILE *fp; @@ -112,26 +136,12 @@ char **rc_config_list (const char *file) char *p; char *token; char **list = NULL; - size_t buflen = BUFSIZ; - size_t last; if (! (fp = fopen (file, "r"))) return (NULL); - buffer = xmalloc (sizeof (char) * buflen); - buffer[buflen - 1] = '\0'; - while (fgets (buffer, buflen, fp)) { - /* Increase the buffer to read the rest of the line if needed */ - last = strlen (buffer) - 1; - while (! feof (fp) && buffer[last] != '\n') { - buflen += BUFSIZ; - buffer = xrealloc (buffer, sizeof (char *) * buflen); - fgets (buffer + last, BUFSIZ, fp); - last = strlen (buffer) - 1; - } - + while ((p = buffer = rc_getline (fp))) { /* Strip leading spaces/tabs */ - p = buffer; while ((*p == ' ') || (*p == '\t')) p++; @@ -144,8 +154,8 @@ char **rc_config_list (const char *file) rc_strlist_add (&list, token); } + free (buffer); } - free (buffer); fclose (fp); return (list); diff --git a/src/librc/librc.c b/src/librc/librc.c index dbabdc03..07652f4d 100644 --- a/src/librc/librc.c +++ b/src/librc/librc.c @@ -307,7 +307,7 @@ char **rc_service_extra_commands (const char *service) char *buffer = NULL; char **commands = NULL; char *token; - char *p = buffer; + char *p; FILE *fp; int l; @@ -319,13 +319,9 @@ char **rc_service_extra_commands (const char *service) snprintf (cmd, l, OPTSTR, svc); free (svc); if ((fp = popen (cmd, "r"))) { - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); - if (fgets (buffer, RC_LINEBUFFER, fp)) { - if (buffer[strlen (buffer) - 1] == '\n') - buffer[strlen (buffer) - 1] = '\0'; - while ((token = strsep (&p, " "))) - rc_strlist_addsort (&commands, token); - } + p = buffer = rc_getline (fp); + while ((token = strsep (&p, " "))) + rc_strlist_addsort (&commands, token); pclose (fp); free (buffer); } @@ -338,12 +334,10 @@ librc_hidden_def(rc_service_extra_commands) char *rc_service_description (const char *service, const char *option) { char *svc; - char *cmd = NULL; - char *buffer; + char *cmd; char *desc = NULL; FILE *fp; int i; - int l; if (! (svc = rc_service_resolve (service))) return (NULL); @@ -351,24 +345,12 @@ char *rc_service_description (const char *service, const char *option) if (! option) option = ""; - l = strlen (DESCSTR) + strlen (svc) + strlen (option) + 2; - cmd = xmalloc (sizeof (char) * l); - snprintf (cmd, l, DESCSTR, svc, option ? "_" : "", option); + i = strlen (DESCSTR) + strlen (svc) + strlen (option) + 2; + cmd = xmalloc (sizeof (char) * i); + snprintf (cmd, i, DESCSTR, svc, option ? "_" : "", option); free (svc); if ((fp = popen (cmd, "r"))) { - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); - while (fgets (buffer, RC_LINEBUFFER, fp)) { - if (! desc) { - desc = xmalloc (strlen (buffer) + 1); - *desc = '\0'; - } else { - desc = xrealloc (desc, strlen (desc) + strlen (buffer) + 1); - } - i = strlen (desc); - memcpy (desc + i, buffer, strlen (buffer)); - memset (desc + i + strlen (buffer), 0, 1); - } - free (buffer); + desc = rc_getline (fp); pclose (fp); } free (cmd); @@ -547,18 +529,17 @@ librc_hidden_def(rc_service_state) char *rc_service_value_get (const char *service, const char *option) { FILE *fp; - char *buffer = NULL; + char *line = NULL; char *file = rc_strcatpaths (RC_SVCDIR, "options", service, option, (char *) NULL); if ((fp = fopen (file, "r"))) { - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); - fgets (buffer, RC_LINEBUFFER, fp); + line = rc_getline (fp); fclose (fp); } free (file); - return (buffer); + return (line); } librc_hidden_def(rc_service_value_get) diff --git a/src/librc/librc.h b/src/librc/librc.h index 32b5000a..491462b8 100644 --- a/src/librc/librc.h +++ b/src/librc/librc.h @@ -83,6 +83,7 @@ librc_hidden_proto(rc_deptree_order) librc_hidden_proto(rc_deptree_update) librc_hidden_proto(rc_deptree_update_needed) librc_hidden_proto(rc_find_pids) +librc_hidden_proto(rc_getline) librc_hidden_proto(rc_runlevel_exists) librc_hidden_proto(rc_runlevel_get) librc_hidden_proto(rc_runlevel_list) diff --git a/src/librc/rc.h b/src/librc/rc.h index 0020764e..fe1213b0 100644 --- a/src/librc/rc.h +++ b/src/librc/rc.h @@ -39,6 +39,7 @@ #include #include +#include /*! @name Reserved runlevel names */ #define RC_LEVEL_SYSINIT "sysinit" @@ -347,6 +348,9 @@ extern FILE *rc_environ_fd; /*! @name Configuration * These functions help to deal with shell based configuration files */ +/*! Return a line from a file, stripping the trailing newline. */ +char *rc_getline (FILE *fp); + /*! Return a NULL terminated list of non comment lines from a file. */ char **rc_config_list (const char *file); diff --git a/src/librc/rc.map b/src/librc/rc.map index e5f8ee34..ff7ef7d1 100644 --- a/src/librc/rc.map +++ b/src/librc/rc.map @@ -12,6 +12,7 @@ global: rc_deptree_update_needed; rc_environ_fd; rc_find_pids; + rc_getline; rc_runlevel_exists; rc_runlevel_get; rc_runlevel_list; diff --git a/src/rc-misc.h b/src/rc-misc.h index c9f699eb..b7208e67 100644 --- a/src/rc-misc.h +++ b/src/rc-misc.h @@ -65,9 +65,6 @@ #define RC_PLUGINDIR RC_LIBDIR "/plugins" -/* Max buffer to read a line from a file */ -#define RC_LINEBUFFER 4096 - #define ERRX fprintf (stderr, "out of memory\n"); exit (1) static inline void *xmalloc (size_t size) diff --git a/src/rc/rc-logger.c b/src/rc/rc-logger.c index 675a4d23..f4c610a2 100644 --- a/src/rc/rc-logger.c +++ b/src/rc/rc-logger.c @@ -181,12 +181,12 @@ void rc_logger_open (const char *level) write_time (log, "started"); else { free (logbuf); - logbuf_size = RC_LINEBUFFER * 10; + logbuf_size = BUFSIZ * 10; logbuf = xmalloc (sizeof (char) * logbuf_size); logbuf_len = 0; } - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); + buffer = xmalloc (sizeof (char) * BUFSIZ); selfd = rc_logger_tty > signal_pipe[0] ? rc_logger_tty : signal_pipe[0]; while (1) { FD_ZERO (&rset); @@ -200,15 +200,15 @@ void rc_logger_open (const char *level) if (s > 0) { if (FD_ISSET (rc_logger_tty, &rset)) { - memset (buffer, 0, RC_LINEBUFFER); - bytes = read (rc_logger_tty, buffer, RC_LINEBUFFER); + memset (buffer, 0, BUFSIZ); + bytes = read (rc_logger_tty, buffer, BUFSIZ); write (STDOUT_FILENO, buffer, bytes); if (log) write_log (fileno (log), buffer, bytes); else { if (logbuf_size - logbuf_len < bytes) { - logbuf_size += RC_LINEBUFFER * 10; + logbuf_size += BUFSIZ * 10; logbuf = xrealloc (logbuf, sizeof (char ) * logbuf_size); } diff --git a/src/rc/rc-misc.c b/src/rc/rc-misc.c index 85d7cd76..79db4323 100644 --- a/src/rc/rc-misc.c +++ b/src/rc/rc-misc.c @@ -238,7 +238,7 @@ char **env_filter (void) static bool file_regex (const char *file, const char *regex) { FILE *fp; - char *buffer; + char *line; regex_t re; bool retval = false; int result; @@ -246,23 +246,22 @@ static bool file_regex (const char *file, const char *regex) if (! (fp = fopen (file, "r"))) return (false); - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); if ((result = regcomp (&re, regex, REG_EXTENDED | REG_NOSUB)) != 0) { fclose (fp); - regerror (result, &re, buffer, RC_LINEBUFFER); - fprintf (stderr, "file_regex: %s", buffer); - free (buffer); + line = xmalloc (sizeof (char) * BUFSIZ); + regerror (result, &re, line, BUFSIZ); + fprintf (stderr, "file_regex: %s", line); + free (line); return (false); } - while (fgets (buffer, RC_LINEBUFFER, fp)) { - if (regexec (&re, buffer, 0, NULL, 0) == 0) - { + while ((line = rc_getline (fp))) { + if (regexec (&re, line, 0, NULL, 0) == 0) retval = true; + free (line); + if (retval) break; - } } - free (buffer); fclose (fp); regfree (&re); diff --git a/src/rc/rc-plugin.c b/src/rc/rc-plugin.c index 613f049e..861f064d 100644 --- a/src/rc/rc-plugin.c +++ b/src/rc/rc-plugin.c @@ -197,10 +197,10 @@ void rc_plugin_run (rc_hook_t hook, const char *value) ssize_t nr; close (pfd[1]); - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); - memset (buffer, 0, RC_LINEBUFFER); + buffer = xmalloc (sizeof (char) * BUFSIZ); + memset (buffer, 0, BUFSIZ); - while ((nr = read (pfd[0], buffer, RC_LINEBUFFER)) > 0) { + while ((nr = read (pfd[0], buffer, BUFSIZ)) > 0) { p = buffer; while (*p && p - buffer < nr) { token = strsep (&p, "="); diff --git a/src/rc/rc.c b/src/rc/rc.c index 6874efce..c94bf11f 100644 --- a/src/rc/rc.c +++ b/src/rc/rc.c @@ -479,7 +479,7 @@ static int do_shell_var (int argc, char **argv) static char *proc_getent (const char *ent) { FILE *fp; - char *buffer; + char *proc; char *p; char *value = NULL; int i; @@ -492,18 +492,11 @@ static char *proc_getent (const char *ent) return (NULL); } - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); - memset (buffer, 0, RC_LINEBUFFER); - if (fgets (buffer, RC_LINEBUFFER, fp) && - (p = strstr (buffer, ent))) + if ((proc = rc_getline (fp)) && + (p = strstr (proc, ent))) { - i = p - buffer; - if (i == '\0' || buffer[i - 1] == ' ') { - /* Trim the trailing carriage return if present */ - i = strlen (buffer) - 1; - if (buffer[i] == '\n') - buffer[i] = 0; - + i = p - proc; + if (i == '\0' || proc[i - 1] == ' ') { p += strlen (ent); if (*p == '=') p++; @@ -511,7 +504,7 @@ static char *proc_getent (const char *ent) } } else errno = ENOENT; - free (buffer); + free (proc); fclose (fp); return (value); diff --git a/src/rc/runscript.c b/src/rc/runscript.c index fdb0496b..0e750c54 100644 --- a/src/rc/runscript.c +++ b/src/rc/runscript.c @@ -451,7 +451,7 @@ static bool svc_exec (const char *arg1, const char *arg2) } selfd = MAX (master_tty, signal_pipe[0]) + 1; - buffer = xmalloc (sizeof (char) * RC_LINEBUFFER); + buffer = xmalloc (sizeof (char) * BUFSIZ); while (1) { FD_ZERO (&rset); FD_SET (signal_pipe[0], &rset); @@ -467,7 +467,7 @@ static bool svc_exec (const char *arg1, const char *arg2) if (s > 0) { if (master_tty >= 0 && FD_ISSET (master_tty, &rset)) { - bytes = read (master_tty, buffer, RC_LINEBUFFER); + bytes = read (master_tty, buffer, BUFSIZ); write_prefix (buffer, bytes, &prefixed); }