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.
diff --git a/arscan.c b/arscan.c
index 549fe1e..6bc5af2 100644
--- a/arscan.c
+++ b/arscan.c
@@ -417,16 +417,14 @@
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 @@
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 @@
/* 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 @@
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 @@
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 @@
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_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_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 @@
)
#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 @@
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 @@
&& (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 @@
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 @@
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 @@
close (desc);
return 0;
+
+ invalid:
+ close (desc);
+ return -2;
}
#endif /* !VMS */
diff --git a/configure.ac b/configure.ac
index 524345e..1187ad4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,7 +132,7 @@
[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 \
diff --git a/function.c b/function.c
index b7f0e56..ebbc88e 100644
--- a/function.c
+++ b/function.c
@@ -1120,6 +1120,7 @@
len += strlen (*argvp) + 2;
p = msg = alloca (len + 1);
+ msg[0] = '\0';
for (argvp=argv; argvp[1] != 0; ++argvp)
{
diff --git a/job.c b/job.c
index 220f637..7d900ce 100644
--- a/job.c
+++ b/job.c
@@ -2151,7 +2151,7 @@
/* 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));
diff --git a/main.c b/main.c
index d5e8b04..e7ad8ca 100644
--- a/main.c
+++ b/main.c
@@ -2172,47 +2172,43 @@
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)
- {
- if (f->deps == 0 && f->cmds != 0)
- {
- /* 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.) */
+ struct file *f;
+ for (f = d->file->double_colon; f != NULL; f = f->prev)
+ if (f->deps == 0 && f->cmds != 0)
+ break;
- DB (DB_VERBOSE,
- (_("Makefile '%s' might loop; not remaking it.\n"),
- f->name));
+ 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.) */
- if (last == 0)
- read_files = d->next;
- else
- last->next = d->next;
+ DB (DB_VERBOSE,
+ (_("Makefile '%s' might loop; not remaking it.\n"),
+ f->name));
- /* Free the storage. */
- free_goaldep (d);
+ if (last)
+ last->next = d->next;
+ else
+ read_files = d->next;
- d = last == 0 ? read_files : last->next;
+ /* Free the storage. */
+ free_goaldep (d);
- break;
- }
- }
-
- if (f == NULL || !f->double_colon)
+ d = last ? last->next : read_files;
+ }
+ else
{
makefile_mtimes = xrealloc (makefile_mtimes,
(mm_idx+1)
diff --git a/maintMakefile b/maintMakefile
index c1d4509..c597804 100644
--- a/maintMakefile
+++ b/maintMakefile
@@ -118,7 +118,6 @@
-$(GIT) clean -fd
-
## ---------------------- ##
## Generating ChangeLog. ##
## ---------------------- ##
@@ -332,6 +331,55 @@
&& 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 @@
esac; \
$(GIT) tag -m "GNU Make release$$message $(VERSION)" -u '$(GPG_FINGERPRINT)' '$(VERSION)'
+
## ------------------------- ##
## GNU FTP upload artifacts. ##
## ------------------------- ##
diff --git a/makeint.h b/makeint.h
index 8f718eb..6d7df4b 100644
--- a/makeint.h
+++ b/makeint.h
@@ -156,11 +156,11 @@
#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
diff --git a/output.c b/output.c
index 65182c4..89bb6d1 100644
--- a/output.c
+++ b/output.c
@@ -52,6 +52,14 @@
# 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 @@
#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 @@
int
output_tmpfd (void)
{
+ MODE_T mask = UMASK (0077);
int fd = -1;
FILE *tfile = tmpfile ();
@@ -300,6 +312,8 @@
set_append_mode (fd);
+ UMASK (mask);
+
return fd;
}
@@ -414,11 +428,15 @@
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 @@
*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 @@
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;
}
diff --git a/read.c b/read.c
index b870aa8..6e2ba7f 100644
--- a/read.c
+++ b/read.c
@@ -2524,7 +2524,7 @@
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;
diff --git a/remake.c b/remake.c
index 5d5d67a..3a908cb 100644
--- a/remake.c
+++ b/remake.c
@@ -353,23 +353,6 @@
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 @@
#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);
