From 3809a8fdebc7087514443871f875d64c9e24e447 Mon Sep 17 00:00:00 2001 From: hubertf Date: Sat, 17 Jul 1999 19:57:03 +0000 Subject: The patch below improves the security of the game atc(6), by having it open the score file at the start and then drop all setgid privileges while keeping a (close-on-exec) file descriptor open to it. In order to allow this the static data files have to be made world readable. In addition a potential buffer overrun with corrupted score files is avoided by more careful use of scanf (note that SCORE_SCANF_FMT is defined alongside the definition of the relevant structure). Submitted in PR 8015 by Joseph Myers --- atc/log.c | 88 +++++++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 63 insertions(+), 25 deletions(-) (limited to 'atc/log.c') diff --git a/atc/log.c b/atc/log.c index 1a515266..746912b4 100644 --- a/atc/log.c +++ b/atc/log.c @@ -1,4 +1,4 @@ -/* $NetBSD: log.c,v 1.8 1998/11/10 13:43:31 hubertf Exp $ */ +/* $NetBSD: log.c,v 1.9 1999/07/17 19:57:03 hubertf Exp $ */ /*- * Copyright (c) 1990, 1993 @@ -50,13 +50,15 @@ #if 0 static char sccsid[] = "@(#)log.c 8.1 (Berkeley) 5/31/93"; #else -__RCSID("$NetBSD: log.c,v 1.8 1998/11/10 13:43:31 hubertf Exp $"); +__RCSID("$NetBSD: log.c,v 1.9 1999/07/17 19:57:03 hubertf Exp $"); #endif #endif not lint #include "include.h" #include "pathnames.h" +static FILE *score_fp; + int compar(va, vb) const void *va, *vb; @@ -101,44 +103,69 @@ timestr(t) return (s); } -int -log_score(list_em) - int list_em; +void +open_score_file() { - int i, fd, num_scores = 0, good, changed = 0, found = 0; - struct passwd *pw; - FILE *fp; - char *cp; - SCORE score[100], thisscore; - struct utsname name; + mode_t old_mask; + int score_fd; + int flags; - umask(0); - fd = open(_PATH_SCORE, O_CREAT|O_RDWR, 0644); - if (fd < 0) { + old_mask = umask(0); + score_fd = open(_PATH_SCORE, O_CREAT|O_RDWR, 0664); + umask(old_mask); + if (score_fd < 0) { warn("open %s", _PATH_SCORE); - return (-1); + return; } + if (score_fd < 3) + exit(1); + /* Set the close-on-exec flag. If this fails for any reason, quit + * rather than leave the score file open to tampering. */ + flags = fcntl(score_fd, F_GETFD); + if (flags < 0) + err(1, "fcntl F_GETFD"); + flags |= FD_CLOEXEC; + if (fcntl(score_fd, F_SETFD, flags) == -1) + err(1, "fcntl F_SETFD"); /* * This is done to take advantage of stdio, while still * allowing a O_CREAT during the open(2) of the log file. */ - fp = fdopen(fd, "r+"); - if (fp == NULL) { + score_fp = fdopen(score_fd, "r+"); + if (score_fp == NULL) { warn("fdopen %s", _PATH_SCORE); + return; + } +} + +int +log_score(list_em) + int list_em; +{ + int i, num_scores = 0, good, changed = 0, found = 0; + struct passwd *pw; + char *cp; + SCORE score[100], thisscore; + struct utsname name; + long offset; + + if (score_fp == NULL) { + warnx("no score file available"); return (-1); } + #ifdef BSD - if (flock(fileno(fp), LOCK_EX) < 0) + if (flock(fileno(score_fp), LOCK_EX) < 0) #endif #ifdef SYSV - while (lockf(fileno(fp), F_LOCK, 1) < 0) + while (lockf(fileno(score_fp), F_LOCK, 1) < 0) #endif { warn("flock %s", _PATH_SCORE); return (-1); } for (;;) { - good = fscanf(fp, "%s %s %s %d %d %d", + good = fscanf(score_fp, SCORE_SCANF_FMT, score[num_scores].name, score[num_scores].host, score[num_scores].game, @@ -152,7 +179,7 @@ log_score(list_em) if ((pw = (struct passwd *) getpwuid(getuid())) == NULL) { fprintf(stderr, "getpwuid failed for uid %d. Who are you?\n", - getuid()); + (int)getuid()); return (-1); } strcpy(thisscore.name, pw->pw_name); @@ -215,12 +242,23 @@ log_score(list_em) else puts("You made the top players list!"); qsort(score, num_scores, sizeof (*score), compar); - rewind(fp); + rewind(score_fp); for (i = 0; i < num_scores; i++) - fprintf(fp, "%s %s %s %d %d %d\n", + fprintf(score_fp, "%s %s %s %d %d %d\n", score[i].name, score[i].host, score[i].game, score[i].planes, score[i].time, score[i].real_time); + fflush(score_fp); + if (ferror(score_fp)) + warn("error writing %s", _PATH_SCORE); + /* It is just possible that updating an entry could + * have reduced the length of the file, so we + * truncate it. The seeks are required for stream/fd + * synchronisation by POSIX.1. */ + offset = ftell(score_fp); + lseek(fileno(score_fp), 0, SEEK_SET); + ftruncate(fileno(score_fp), offset); + rewind(score_fp); } else { if (found) puts("You didn't beat your previous score."); @@ -230,12 +268,12 @@ log_score(list_em) putchar('\n'); } #ifdef BSD - flock(fileno(fp), LOCK_UN); + flock(fileno(score_fp), LOCK_UN); #endif #ifdef SYSV /* lock will evaporate upon close */ #endif - fclose(fp); + fclose(score_fp); printf("%2s: %-8s %-8s %-18s %4s %9s %4s\n", "#", "name", "host", "game", "time", "real time", "planes safe"); puts("-------------------------------------------------------------------------------"); -- cgit v1.2.3