[PATCH] Buffer over-run fix for getusershell(3)

[David Stacey]

This is the first patch resulting from the Coverity Scan analysis of the 
Cygwin source code. The patch fixes Coverity ID 59932. Note that we 
don't have that many bugs in the Cygwin source code - that's just an ID 
that Coverity assigned to this issue. The patch is only a single line, 
so it falls into our definition of 'trivial'.

getusershell(3) returns the next line from the '/etc/shells' file [1]. 
This contains a path to an executable, so it makes sense for 'buf' to 
contain PATH_MAX characters.

Now, the definition of PATH_MAX is the maximum length of the path, 
including the null terminator [2]. So the for() loop should copy 
PATH_MAX-1 characters, in order to allow for the null terminator.

However, by copying PATH_MAX characters, there is a possible buffer 
over-run when the null terminator is applied. The patch (attached) 
corrects this.

Change Log:
2014-05-18  David Stacey  <drstacey@tiscali.co.uk>

         * winsup/cygwin/syscalls.cc(getusershell) :
         Fixed theoretical buffer overrun of 'buf' that would occur if
         /etc/shells contained a line longer than 4095 characters.

Cheers,

Dave.

[1] http://linux.die.net/man/3/getusershell
[2] http://pubs.opengroup.org/onlinepubs/009695399/basedefs/limits.h.html

-------------- next part --------------
--- cygwin-orig-src/winsup/cygwin/syscalls.cc	2014-05-14 12:29:12.000000000 +0100
+++ cygwin-src/winsup/cygwin/syscalls.cc	2014-05-18 19:43:18.355953300 +0100
@@ -4179,7 +4179,7 @@
   /* Get each non-whitespace character as part of the shell path as long as
      it fits in buf. */
   for (buf_idx = 0;
-       ch != EOF && !isspace (ch) && buf_idx < PATH_MAX;
+       ch != EOF && !isspace (ch) && buf_idx < (PATH_MAX - 1);
        buf_idx++, ch = getc (shell_fp))
     buf[buf_idx] = ch;
   /* Skip any trailing non-whitespace character not fitting in buf.  If the