Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 7 Jun 2016 16:56:15 +0000 (UTC)
From:      Don Lewis <truckman@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r301557 - stable/10/usr.sbin/pw
Message-ID:  <201606071656.u57GuFr9057462@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: truckman
Date: Tue Jun  7 16:56:15 2016
New Revision: 301557
URL: https://svnweb.freebsd.org/changeset/base/301557

Log:
  MFC r300564
  
  Fix CID 1006692 in /usr/sbin/pw pw_log() function and other fixes
  
  The length of the name returned from the $LOGNAME and $USER can be
  very long and it was being concatenated to a fixed length buffer
  with no bounds checking.  Fix this problem by limiting the length
  of the name copied.
  
  Additionally, this name is actually used to create a format string
  to be used in adding log file entries so embedded % characters in
  the name could confuse *printf(), and embedded whitespace could
  confuse a log file parser.  Handle the former by escaping each %
  with an additional %, and handle the latter by simply stripping it
  out.
  
  Clean up the code by moving the variable declarations to the top
  of the function, formatting them to conform with style, and moving
  intialization elsewhere.
  
  Reduce code indentation by returning early in a couple of places.
  
  Reported by:	Coverity
  CID:		1006692
  Reviewed by:	markj (previous version)
  Differential Revision:	https://reviews.freebsd.org/D6490

Modified:
  stable/10/usr.sbin/pw/pw_log.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/usr.sbin/pw/pw_log.c
==============================================================================
--- stable/10/usr.sbin/pw/pw_log.c	Tue Jun  7 16:53:05 2016	(r301556)
+++ stable/10/usr.sbin/pw/pw_log.c	Tue Jun  7 16:56:15 2016	(r301557)
@@ -29,40 +29,90 @@ static const char rcsid[] =
   "$FreeBSD$";
 #endif /* not lint */
 
+#include <ctype.h>
+#include <err.h>
 #include <fcntl.h>
 #include <string.h>
 #include <stdarg.h>
 
 #include "pw.h"
 
-static FILE    *logfile = NULL;
+static FILE	*logfile = NULL;
 
 void
 pw_log(struct userconf * cnf, int mode, int which, char const * fmt,...)
 {
-	if (cnf->logfile && *cnf->logfile) {
-		if (logfile == NULL) {	/* With umask==0 we need to control file access modes on create */
-			int             fd = open(cnf->logfile, O_WRONLY | O_CREAT | O_APPEND, 0600);
+	va_list		argp;
+	time_t		now;
+	const char	*cp, *name;
+	struct tm	*t;
+	int		fd, i, rlen;
+	char		nfmt[256], sname[32];
 
-			if (fd != -1)
-				logfile = fdopen(fd, "a");
+	if (cnf->logfile == NULL || cnf->logfile[0] == '\0') {
+		return;
+	}
+
+	if (logfile == NULL) {
+		/* With umask==0 we need to control file access modes on create */
+		fd = open(cnf->logfile, O_WRONLY | O_CREAT | O_APPEND, 0600);
+		if (fd == -1) {
+			return;
 		}
-		if (logfile != NULL) {
-			va_list         argp;
-			time_t          now = time(NULL);
-			struct tm      *t = localtime(&now);
-			char            nfmt[256];
-			const char     *name;
-
-			if ((name = getenv("LOGNAME")) == NULL && (name = getenv("USER")) == NULL)
-				name = "unknown";
-			/* ISO 8601 International Standard Date format */
-			strftime(nfmt, sizeof nfmt, "%Y-%m-%d %T ", t);
-			sprintf(nfmt + strlen(nfmt), "[%s:%s%s] %s\n", name, Which[which], Modes[mode], fmt);
-			va_start(argp, fmt);
-			vfprintf(logfile, nfmt, argp);
-			va_end(argp);
-			fflush(logfile);
+		logfile = fdopen(fd, "a");
+		if (logfile == NULL) {
+			return;
 		}
 	}
+
+	if ((name = getenv("LOGNAME")) == NULL &&
+	    (name = getenv("USER")) == NULL) {
+		strcpy(sname, "unknown");
+	} else {
+		/*
+		 * Since "name" will be embedded in a printf-like format,
+		 * we must sanitize it:
+		 *
+		 *    Limit its length so other information in the message
+		 *    is not truncated
+		 *
+		 *    Squeeze out embedded whitespace for the benefit of
+		 *    log file parsers
+		 *
+		 *    Escape embedded % characters with another %
+		 */
+		for (i = 0, cp = name;
+		    *cp != '\0' && i < (int)sizeof(sname) - 1; cp++) {
+			if (*cp == '%') {
+				if (i < (int)sizeof(sname) - 2) {
+					sname[i++] = '%';
+					sname[i++] = '%';
+				} else {
+					break;
+				}
+			} else if (!isspace(*cp)) {
+				sname[i++] = *cp;
+			} /* else do nothing */
+		}
+		if (i == 0) {
+			strcpy(sname, "unknown");
+		} else {
+			sname[i] = '\0';
+		}
+	}
+	now = time(NULL);
+	t = localtime(&now);
+	/* ISO 8601 International Standard Date format */
+	strftime(nfmt, sizeof nfmt, "%Y-%m-%d %T ", t);
+	rlen = sizeof(nfmt) - strlen(nfmt);
+	if (rlen <= 0 || snprintf(nfmt + strlen(nfmt), rlen,
+	    "[%s:%s%s] %s\n", sname, Which[which], Modes[mode],
+	    fmt) >= rlen) {
+		warnx("log format overflow, user name=%s", sname);
+	} else {
+		va_start(argp, fmt);
+		vfprintf(logfile, nfmt, argp);
+		va_end(argp);
+		fflush(logfile);
+	}
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201606071656.u57GuFr9057462>