Resolve issues discovered by static code analysis.

* maintMakefile: Add a rule to submit code for analysis.
* configure.ac: Check for availability of the umask() function.
* output.c (output_tmpfd, output_tmpfile): Use umask on temp files.
* makeint.h (PATH_VAR): Reserve an extra character for nul bytes.
* function.c (func_error): Initialize buffer to empty string.
* job.c (child_execute_job): Verify validity of fdin.
* main.c (main): Simplify code for makefile updating algorithm.
* arscan.c (ar_scan): Verify member name length before reading.
* read.c (readline): Cast pointer arithmetic to avoid warnings.
* remake.c (update_file): Remove unreachable code.
(name_mtime): Verify symlink name length.
This commit is contained in:
Paul Smith 2016-09-25 19:06:56 -04:00
parent d3bba301ce
commit bc9d72beb0
10 changed files with 161 additions and 134 deletions

119
arscan.c
View File

@ -417,16 +417,14 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
int desc = open (archive, O_RDONLY, 0);
if (desc < 0)
return -1;
#ifdef SARMAG
{
char buf[SARMAG];
int nread;
EINTRLOOP (nread, read (desc, buf, SARMAG));
if (nread != SARMAG || memcmp (buf, ARMAG, SARMAG))
{
(void) close (desc);
return -2;
}
goto invalid;
}
#else
#ifdef AIAMAG
@ -434,10 +432,8 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
int nread;
EINTRLOOP (nread, read (desc, &fl_header, FL_HSZ));
if (nread != FL_HSZ)
{
(void) close (desc);
return -2;
}
goto invalid;
#ifdef AIAMAGBIG
/* If this is a "big" archive, then set the flag and
re-read the header into the "big" structure. */
@ -450,27 +446,18 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
/* seek back to beginning of archive */
EINTRLOOP (o, lseek (desc, 0, 0));
if (o < 0)
{
(void) close (desc);
return -2;
}
goto invalid;
/* re-read the header into the "big" structure */
EINTRLOOP (nread, read (desc, &fl_header_big, FL_HSZ_BIG));
if (nread != FL_HSZ_BIG)
{
(void) close (desc);
return -2;
}
goto invalid;
}
else
#endif
/* Check to make sure this is a "normal" archive. */
if (memcmp (fl_header.fl_magic, AIAMAG, SAIAMAG))
{
(void) close (desc);
return -2;
}
goto invalid;
}
#else
{
@ -482,10 +469,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
int nread;
EINTRLOOP (nread, read (desc, &buf, sizeof (buf)));
if (nread != sizeof (buf) || buf != ARMAG)
{
(void) close (desc);
return -2;
}
goto invalid;
}
#endif
#endif
@ -535,13 +519,15 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
struct ar_hdr_big member_header_big;
#endif
#ifdef AIAMAG
char name[256];
# define ARNAME_MAX 255
char name[ARNAME_MAX + 1];
int name_len;
long int dateval;
int uidval, gidval;
long int data_offset;
#else
char namebuf[sizeof member_header.ar_name + 1];
# define ARNAME_MAX (int)sizeof(member_header.ar_name)
char namebuf[ARNAME_MAX + 1];
char *name;
int is_namemap; /* Nonzero if this entry maps long names. */
int long_name = 0;
@ -553,10 +539,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
EINTRLOOP (o, lseek (desc, member_offset, 0));
if (o < 0)
{
(void) close (desc);
return -2;
}
goto invalid;
#ifdef AIAMAG
#define AR_MEMHDR_SZ(x) (sizeof(x) - sizeof (x._ar_name))
@ -568,21 +551,17 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
AR_MEMHDR_SZ(member_header_big)));
if (nread != AR_MEMHDR_SZ(member_header_big))
{
(void) close (desc);
return -2;
}
goto invalid;
sscanf (member_header_big.ar_namlen, "%4d", &name_len);
if (name_len < 1 || name_len > ARNAME_MAX)
goto invalid;
EINTRLOOP (nread, read (desc, name, name_len));
if (nread != name_len)
{
(void) close (desc);
return -2;
}
goto invalid;
name[name_len] = 0;
name[name_len] = '\0';
sscanf (member_header_big.ar_date, "%12ld", &dateval);
sscanf (member_header_big.ar_uid, "%12d", &uidval);
@ -600,21 +579,17 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
AR_MEMHDR_SZ(member_header)));
if (nread != AR_MEMHDR_SZ(member_header))
{
(void) close (desc);
return -2;
}
goto invalid;
sscanf (member_header.ar_namlen, "%4d", &name_len);
if (name_len < 1 || name_len > ARNAME_MAX)
goto invalid;
EINTRLOOP (nread, read (desc, name, name_len));
if (nread != name_len)
{
(void) close (desc);
return -2;
}
goto invalid;
name[name_len] = 0;
name[name_len] = '\0';
sscanf (member_header.ar_date, "%12ld", &dateval);
sscanf (member_header.ar_uid, "%12d", &uidval);
@ -656,10 +631,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
)
#endif
)
{
(void) close (desc);
return -2;
}
goto invalid;
name = namebuf;
memcpy (name, member_header.ar_name, sizeof member_header.ar_name);
@ -679,6 +651,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
is_namemap = (!strcmp (name, "//")
|| !strcmp (name, "ARFILENAMES/"));
#endif /* Not AIAMAG. */
/* On some systems, there is a slash after each member name. */
if (*p == '/')
*p = '\0';
@ -693,23 +666,27 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
&& (name[0] == ' ' || name[0] == '/')
&& namemap != 0)
{
name = namemap + atoi (name + 1);
int name_off = atoi (name + 1);
if (name_off < 1 || name_off > ARNAME_MAX)
goto invalid;
name = namemap + name_off;
long_name = 1;
}
else if (name[0] == '#'
&& name[1] == '1'
&& name[2] == '/')
{
int namesize = atoi (name + 3);
int name_len = atoi (name + 3);
if (name_len < 1 || name_len > ARNAME_MAX)
goto invalid;
name = alloca (namesize + 1);
EINTRLOOP (nread, read (desc, name, namesize));
if (nread != namesize)
{
close (desc);
return -2;
}
name[namesize] = '\0';
name = alloca (name_len + 1);
EINTRLOOP (nread, read (desc, name, name_len));
if (nread != name_len)
goto invalid;
name[name_len] = '\0';
long_name = 1;
}
@ -759,10 +736,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
sscanf (member_header.ar_nxtmem, "%12ld", &member_offset);
if (lseek (desc, member_offset, 0) != member_offset)
{
(void) close (desc);
return -2;
}
goto invalid;
#else
/* If this member maps archive names, we must read it in. The
@ -776,10 +750,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
namemap = alloca (eltsize);
EINTRLOOP (nread, read (desc, namemap, eltsize));
if (nread != eltsize)
{
(void) close (desc);
return -2;
}
goto invalid;
/* The names are separated by newlines. Some formats have
a trailing slash. Null terminate the strings for
@ -807,6 +778,10 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
close (desc);
return 0;
invalid:
close (desc);
return -2;
}
#endif /* !VMS */

View File

@ -132,7 +132,7 @@ AS_IF([test "$ac_cv_func_gettimeofday" = yes],
[Define to 1 if you have a standard gettimeofday function])
])
AC_CHECK_FUNCS([strdup strndup mkstemp mktemp fdopen fileno \
AC_CHECK_FUNCS([strdup strndup umask mkstemp mktemp fdopen fileno \
dup dup2 getcwd realpath sigsetmask sigaction \
getgroups seteuid setegid setlinebuf setreuid setregid \
getrlimit setrlimit setvbuf pipe strerror strsignal \

View File

@ -1120,6 +1120,7 @@ func_error (char *o, char **argv, const char *funcname)
len += strlen (*argvp) + 2;
p = msg = alloca (len + 1);
msg[0] = '\0';
for (argvp=argv; argvp[1] != 0; ++argvp)
{

2
job.c
View File

@ -2151,7 +2151,7 @@ child_execute_job (struct output *out, int good_stdin, char **argv, char **envp)
/* For any redirected FD, dup2() it to the standard FD.
They are all marked close-on-exec already. */
if (fdin != FD_STDIN)
if (fdin >= 0 && fdin != FD_STDIN)
EINTRLOOP (r, dup2 (fdin, FD_STDIN));
if (fdout != FD_STDOUT)
EINTRLOOP (r, dup2 (fdout, FD_STDOUT));

40
main.c
View File

@ -2172,47 +2172,43 @@ main (int argc, char **argv, char **envp)
DB (DB_BASIC, (_("Updating makefiles....\n")));
/* Remove any makefiles we don't want to try to update.
Also record the current modtimes so we can compare them later. */
/* Remove any makefiles we don't want to try to update. Record the
current modtimes of the others so we can compare them later. */
{
register struct goaldep *d, *last;
last = 0;
d = read_files;
while (d != 0)
{
struct file *f = d->file;
if (f->double_colon)
for (f = f->double_colon; f != NULL; f = f->prev)
{
struct file *f;
for (f = d->file->double_colon; f != NULL; f = f->prev)
if (f->deps == 0 && f->cmds != 0)
break;
if (f)
{
/* This makefile is a :: target with commands, but
no dependencies. So, it will always be remade.
This might well cause an infinite loop, so don't
try to remake it. (This will only happen if
your makefiles are written exceptionally
stupidly; but if you work for Athena, that's how
you write your makefiles.) */
/* This makefile is a :: target with commands, but no
dependencies. So, it will always be remade. This might
well cause an infinite loop, so don't try to remake it.
(This will only happen if your makefiles are written
exceptionally stupidly; but if you work for Athena, that's
how you write your makefiles.) */
DB (DB_VERBOSE,
(_("Makefile '%s' might loop; not remaking it.\n"),
f->name));
if (last == 0)
read_files = d->next;
else
if (last)
last->next = d->next;
else
read_files = d->next;
/* Free the storage. */
free_goaldep (d);
d = last == 0 ? read_files : last->next;
break;
d = last ? last->next : read_files;
}
}
if (f == NULL || !f->double_colon)
else
{
makefile_mtimes = xrealloc (makefile_mtimes,
(mm_idx+1)

View File

@ -118,7 +118,6 @@ git-very-clean: git-clean
-$(GIT) clean -fd
## ---------------------- ##
## Generating ChangeLog. ##
## ---------------------- ##
@ -332,6 +331,55 @@ gendocs: update-gnuweb update-makeweb
&& echo '- cvs commit' \
&& echo '- cvs tag make-$(subst .,-,$(VERSION))'
## --------------------------------------------- ##
## Submitting Coverity cov-build results to Scan ##
## --------------------------------------------- ##
# Note you must have set COVERITY_TOKEN and COVERITY_EMAIL properly
# to submit results. COVERITY_PATH can be set to the root of the
# cov-build tools if it's not already on your PATH.
COV_BUILD_FILE := cov-build.tgz
.PHONY: cov-build cov-submit
cov-build: $(COV_BUILD_FILE)
$(COV_BUILD_FILE): $(filter %.c %.h,$(DISTFILES))
$(MAKE) distdir
@echo "Running Coverity cov-build"
rm -rf '$(distdir)'/_build
mkdir '$(distdir)'/_build
cd '$(distdir)'/_build \
&& ../configure --srcdir=.. \
$(AM_DISTCHECK_CONFIGURE_FLAGS) $(DISTCHECK_CONFIGURE_FLAGS) \
CFLAGS='$(AM_CFLAGS)'
PATH="$${COVERITY_PATH:+$$COVERITY_PATH/bin:}$$PATH"; \
cd '$(distdir)'/_build \
&& cov-build --dir cov-int ./build.sh
rm -f '$@'
(cd '$(distdir)'/_build && tar czf - cov-int) > '$@'
cov-submit: $(COV_BUILD_FILE)-submitted
$(COV_BUILD_FILE)-submitted: $(COV_BUILD_FILE)
@[ -n "$(COVERITY_TOKEN)" ] || { echo 'COVERITY_TOKEN not set'; exit 1; }
@[ -n "$(COVERITY_EMAIL)" ] || { echo 'COVERITY_EMAIL not set'; exit 1; }
rm -f '$@'
case '$(VERSION)' in \
(*.*.9*) type="daily build"; ext=".$$(date +%Y%m%d)" ;; \
(*) type="release"; ext= ;; \
esac; \
curl --form token='$(COVERITY_TOKEN)' \
--form email='$(COVERITY_EMAIL)' \
--form file='@$<' \
--form version="$(VERSION)$$ext" \
--form description="GNU make $$type" \
'https://scan.coverity.com/builds?project=gmake'
cp '$<' '$@'
## ------------------------- ##
## Make release targets. ##
## ------------------------- ##
@ -344,6 +392,7 @@ tag-release:
esac; \
$(GIT) tag -m "GNU Make release$$message $(VERSION)" -u '$(GPG_FINGERPRINT)' '$(VERSION)'
## ------------------------- ##
## GNU FTP upload artifacts. ##
## ------------------------- ##

View File

@ -156,11 +156,11 @@ extern int errno;
#ifdef PATH_MAX
# define GET_PATH_MAX PATH_MAX
# define PATH_VAR(var) char var[PATH_MAX]
# define PATH_VAR(var) char var[PATH_MAX+1]
#else
# define NEED_GET_PATH_MAX 1
# define GET_PATH_MAX (get_path_max ())
# define PATH_VAR(var) char *var = alloca (GET_PATH_MAX)
# define PATH_VAR(var) char *var = alloca (GET_PATH_MAX+1)
unsigned int get_path_max (void);
#endif

View File

@ -52,6 +52,14 @@ unsigned int stdio_traced = 0;
# define STREAM_OK(_s) 1
#endif
#if defined(HAVE_UMASK)
# define UMASK(_m) umask (_m)
# define MODE_T mode_t
#else
# define UMASK(_m) 0
# define MODE_T int
#endif
/* Write a string to the current STDOUT or STDERR. */
static void
_outputs (struct output *out, int is_err, const char *msg)
@ -160,7 +168,10 @@ set_append_mode (int fd)
#if defined(F_GETFL) && defined(F_SETFL) && defined(O_APPEND)
int flags = fcntl (fd, F_GETFL, 0);
if (flags >= 0)
fcntl (fd, F_SETFL, flags | O_APPEND);
{
int r;
EINTRLOOP(r, fcntl (fd, F_SETFL, flags | O_APPEND));
}
#endif
}
@ -285,6 +296,7 @@ release_semaphore (void *sem)
int
output_tmpfd (void)
{
MODE_T mask = UMASK (0077);
int fd = -1;
FILE *tfile = tmpfile ();
@ -300,6 +312,8 @@ output_tmpfd (void)
set_append_mode (fd);
UMASK (mask);
return fd;
}
@ -414,11 +428,15 @@ char *mktemp (char *template);
FILE *
output_tmpfile (char **name, const char *template)
{
FILE *file;
#ifdef HAVE_FDOPEN
int fd;
#endif
#if defined HAVE_MKSTEMP || defined HAVE_MKTEMP
/* Preserve the current umask, and set a restrictive one for temp files. */
MODE_T mask = UMASK (0077);
#if defined(HAVE_MKSTEMP) || defined(HAVE_MKTEMP)
# define TEMPLATE_LEN strlen (template)
#else
# define TEMPLATE_LEN L_tmpnam
@ -426,12 +444,13 @@ output_tmpfile (char **name, const char *template)
*name = xmalloc (TEMPLATE_LEN + 1);
strcpy (*name, template);
#if defined HAVE_MKSTEMP && defined HAVE_FDOPEN
#if defined(HAVE_MKSTEMP) && defined(HAVE_FDOPEN)
/* It's safest to use mkstemp(), if we can. */
fd = mkstemp (*name);
EINTRLOOP (fd, mkstemp (*name));
if (fd == -1)
return 0;
return fdopen (fd, "w");
file = NULL;
else
file = fdopen (fd, "w");
#else
# ifdef HAVE_MKTEMP
(void) mktemp (*name);
@ -444,12 +463,16 @@ output_tmpfile (char **name, const char *template)
EINTRLOOP (fd, open (*name, O_CREAT|O_EXCL|O_WRONLY, 0600));
if (fd == -1)
return 0;
return fdopen (fd, "w");
file = fdopen (fd, "w");
# else
/* Not secure, but what can we do? */
return fopen (*name, "w");
file = fopen (*name, "w");
# endif
#endif
UMASK (mask);
return file;
}

2
read.c
View File

@ -2524,7 +2524,7 @@ readline (struct ebuffer *ebuf)
end = p + ebuf->size;
*p = '\0';
while (fgets (p, end - p, ebuf->fp) != 0)
while (fgets (p, (int)(end - p), ebuf->fp) != 0)
{
char *p2;
unsigned long len;

View File

@ -353,23 +353,6 @@ update_file (struct file *file, unsigned int depth)
status = new;
}
/* Process the remaining rules in the double colon chain so they're marked
considered. Start their prerequisites, too. */
if (file->double_colon)
for (; f != 0 ; f = f->prev)
{
struct dep *d;
f->considered = considered;
for (d = f->deps; d != 0; d = d->next)
{
enum update_status new = update_file (d->file, depth + 1);
if (new > status)
status = new;
}
}
return status;
}
@ -1510,7 +1493,7 @@ name_mtime (const char *name)
#ifndef S_ISLNK
# define S_ISLNK(_m) (((_m)&S_IFMT)==S_IFLNK)
#endif
if (check_symlink_flag)
if (check_symlink_flag && strlen (name) <= GET_PATH_MAX)
{
PATH_VAR (lpath);