d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix CID 1006692 in /usr/sbin/pw pw_log() function and other fixes

Browse Source 
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)
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D6490
This commit is contained in:
Don Lewis 2016-05-24 05:02:24 +00:00
parent 40dcddfcca
commit 660d4fe299
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=300564
1 changed files with 72 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

94
usr.sbin/pw/pw_log.c
View File

@ -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);
}
}