diff --git a/doc/Changelog b/doc/Changelog index 36601ec..36624c9 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -21,6 +21,17 @@ sysvinit (2.94) unreleased; urgency=low * Added logsave.c and logsave.8 manual page from e2fsprogs to make sure logsave is available to initscripts. * Updated src/Makefile to make sure bootlogd compiles with Clang. + * Use defined constants for password length in sulogin. Makes + it easier to update/patch later. + * Minor code fixes across multiple source files to avoid buffer + overflows, or uninitialized strings. + * Changed the way the "when" variable is used internally in shutdown.c. + It starts as a NULL pointer, then might get set as a pointer to optarg, + then it might get set to point to an argv parameter, then it might have + a string value copied into it, over-writing the original data. We should + not risk over-writing internal variables which might get used for something + else (it's rude and security risk). Set up "when" as its own buffer + that has data from optargs and/or argv copied into it. sysvinit (2.93) released; urgency=low diff --git a/src/killall5.c b/src/killall5.c index d8a399e..8f2ad50 100644 --- a/src/killall5.c +++ b/src/killall5.c @@ -433,7 +433,11 @@ int check4nfs(const char * path, char * real) } while (1); - if (real) strcpy(real, curr); + if (real) /* real is defined elsewhere as being PATH_MAX + 1 */ + { + memset(real, '\0', PATH_MAX + 1); + strncpy(real, curr, PATH_MAX); + } if (errno == EINVAL) { const size_t nlen = strlen(curr); diff --git a/src/last.c b/src/last.c index 5252015..1c028cc 100644 --- a/src/last.c +++ b/src/last.c @@ -48,6 +48,10 @@ # define SHUTDOWN_TIME 254 #endif +#ifndef PATH_MAX +#define PATH_MAX 2048 +#endif + char *Version = "@(#) last 2.85 31-Apr-2004 miquels"; #define CHOP_DOMAIN 0 /* Define to chop off local domainname. */ @@ -253,10 +257,11 @@ int uread(FILE *fp, struct utmp *u, int *quit) #define BTMP_FILE getbtmp() char *getbtmp() { - static char btmp[128]; + static char btmp[PATH_MAX + 5]; /* max path + btmp + null terminator */ char *p; - strcpy(btmp, WTMP_FILE); + memset(btmp, '\0', PATH_MAX + 5); + strncpy(btmp, WTMP_FILE, PATH_MAX); if ((p = strrchr(btmp, '/')) == NULL) p = btmp; else @@ -841,7 +846,7 @@ int main(int argc, char **argv) switch (ut.ut_type) { case SHUTDOWN_TIME: if (extended) { - strcpy(ut.ut_line, "system down"); + strncpy(ut.ut_line, "system down", OLD_LINESIZE - 1); quit = list(&ut, lastboot, R_NORMAL); } lastdown = lastrch = ut.ut_time; @@ -850,14 +855,14 @@ int main(int argc, char **argv) case OLD_TIME: case NEW_TIME: if (extended) { - strcpy(ut.ut_line, + strncpy(ut.ut_line, ut.ut_type == NEW_TIME ? "new time" : - "old time"); + "old time", OLD_LINESIZE - 1); quit = list(&ut, lastdown, R_TIMECHANGE); } break; case BOOT_TIME: - strcpy(ut.ut_line, "system boot"); + strncpy(ut.ut_line, "system boot", OLD_LINESIZE - 1); quit = list(&ut, lastdown, R_REBOOT); lastboot = ut.ut_time; down = 1; diff --git a/src/logsave.c b/src/logsave.c index 0c9b1fd..1c52245 100644 --- a/src/logsave.c +++ b/src/logsave.c @@ -282,7 +282,7 @@ int main(int argc, char **argv) outfn = argv[optind]; optind++; argv += optind; - argc -= optind; + /* argc -= optind; - this is not used */ outfd = open(outfn, openflags, 0644); do_stdin = !strcmp(argv[0], "-"); diff --git a/src/mountpoint.c b/src/mountpoint.c index 3daf470..b24335e 100644 --- a/src/mountpoint.c +++ b/src/mountpoint.c @@ -32,6 +32,10 @@ #include #include +#ifndef PATH_MAX +#define PATH_MAX 2048 +#endif + int dostat(char *path, struct stat *st, int do_lstat, int quiet) { int n; @@ -105,7 +109,7 @@ void usage(void) { int main(int argc, char **argv) { struct stat st, st2; - char buf[256]; + char buf[PATH_MAX + 1]; char *path; int quiet = 0; int showdev = 0; @@ -163,7 +167,7 @@ int main(int argc, char **argv) memset(buf, 0, sizeof(buf)); strncpy(buf, path, sizeof(buf) - 4); - strcat(buf, "/.."); + strncat(buf, "/..", 3); if (dostat(buf, &st2, 0, quiet) < 0) return 1; diff --git a/src/shutdown.c b/src/shutdown.c index 0e6f450..b744a2c 100644 --- a/src/shutdown.c +++ b/src/shutdown.c @@ -70,6 +70,8 @@ extern char **environ; #endif #define MESSAGELEN 256 +#define STATELEN 64 +#define WHEN_SIZE 64 /* Whether we should warn system is shutting down */ #define QUIET_FULL 2 @@ -83,7 +85,7 @@ int fastboot = 0; /* Do a 'fast' reboot */ int forcefsck = 0; /* Force fsck on reboot */ char message[MESSAGELEN]; /* Warning message */ char *sltime = 0; /* Sleep time */ -char newstate[64]; /* What are we gonna do */ +char newstate[STATELEN]; /* What are we gonna do */ int doself = 0; /* Don't use init */ int got_alrm = 0; @@ -232,11 +234,11 @@ int init_setenv(char *name, char *value) */ void issue_warn(int mins) { - char buf[MESSAGELEN + sizeof(newstate)]; + char buf[MESSAGELEN + sizeof(newstate) + 1]; int len; buf[0] = 0; - strncat(buf, message, sizeof(buf) - 1); + strncpy(buf, message, MESSAGELEN); len = strlen(buf); if (mins == 0) @@ -519,7 +521,7 @@ int main(int argc, char **argv) char buf[128]; char term[UT_LINESIZE + 6]; char *sp; - char *when = NULL; + char when[WHEN_SIZE]; int c, i, wt; int hours, mins; int didnolog = 0; @@ -547,6 +549,7 @@ int main(int argc, char **argv) } strcpy(down_level, "1"); halttype = NULL; + memset(when, '\0', WHEN_SIZE); /* Process the options. */ while((c = getopt(argc, argv, "HPacqQkrhnfFyt:g:i:")) != EOF) { @@ -593,7 +596,7 @@ int main(int argc, char **argv) case 'y': /* Ignored for sysV compatibility */ break; case 'g': /* sysv style to specify time. */ - when = optarg; + strncpy(when, optarg, WHEN_SIZE - 1); break; case 'i': /* Level to go to. */ if (!strchr("0156aAbBcCsS", optarg[0])) { @@ -679,7 +682,7 @@ int main(int argc, char **argv) /* Read remaining words, skip time if needed. */ message[0] = 0; - for(c = optind + (!cancel && !when); c < argc; c++) { + for(c = optind + (!cancel && !when[0]); c < argc; c++) { if (strlen(message) + strlen(argv[c]) + 4 > MESSAGELEN) break; strcat(message, argv[c]); @@ -704,9 +707,9 @@ int main(int argc, char **argv) } /* Check syntax. */ - if (when == NULL) { + if (when[0] == '\0') { if (optind == argc) usage(); - when = argv[optind++]; + strncpy(when, argv[optind++], WHEN_SIZE - 1); } /* See if we are already running. */ @@ -725,16 +728,16 @@ int main(int argc, char **argv) /* Tell users what we're gonna do. */ switch(down_level[0]) { case '0': - strcpy(newstate, "for system halt"); + strncpy(newstate, "for system halt", STATELEN); break; case '6': - strcpy(newstate, "for reboot"); + strncpy(newstate, "for reboot", STATELEN); break; case '1': - strcpy(newstate, "to maintenance mode"); + strncpy(newstate, "to maintenance mode", STATELEN); break; default: - sprintf(newstate, "to runlevel %s", down_level); + snprintf(newstate, STATELEN, "to runlevel %s", down_level); break; } @@ -772,10 +775,11 @@ int main(int argc, char **argv) /* Alias now and take care of old '+mins' notation. */ if (!strcmp(when, "now")) strcpy(when, "0"); - if (when[0] == '+') when++; + sp = when; + if (when[0] == '+') sp++; /* Decode shutdown time. */ - for (sp = when; *sp; sp++) { + for ( ; *sp; sp++) { if (*sp != ':' && (*sp < '0' || *sp > '9')) usage(); }