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 <drsta...@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
--- 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
