Pino Toscano
2014-Aug-08 15:21 UTC
[Libguestfs] [PATCH] fish: do not overflow when copying the socket path
--- fish/rc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fish/rc.c b/fish/rc.c index 8e22c53..c066d98 100644 --- a/fish/rc.c +++ b/fish/rc.c @@ -82,7 +82,8 @@ create_sockpath (pid_t pid, char *sockpath, size_t len, snprintf (sockpath, len, SOCKET_PATH, euid, pid); addr->sun_family = AF_UNIX; - strcpy (addr->sun_path, sockpath); + strncpy (addr->sun_path, sockpath, UNIX_PATH_MAX); + addr->sun_path[UNIX_PATH_MAX-1] = '\0'; } static const socklen_t controllen = CMSG_LEN (sizeof (int)); -- 1.9.3
Richard W.M. Jones
2014-Aug-08 18:38 UTC
Re: [Libguestfs] [PATCH] fish: do not overflow when copying the socket path
On Fri, Aug 08, 2014 at 05:21:25PM +0200, Pino Toscano wrote:> --- > fish/rc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fish/rc.c b/fish/rc.c > index 8e22c53..c066d98 100644 > --- a/fish/rc.c > +++ b/fish/rc.c > @@ -82,7 +82,8 @@ create_sockpath (pid_t pid, char *sockpath, size_t len, > snprintf (sockpath, len, SOCKET_PATH, euid, pid); > > addr->sun_family = AF_UNIX; > - strcpy (addr->sun_path, sockpath); > + strncpy (addr->sun_path, sockpath, UNIX_PATH_MAX); > + addr->sun_path[UNIX_PATH_MAX-1] = '\0';I suspect we're going to get in trouble here for using strncpy, which is a well-known "red flag" for security. The specific problem here (although to be fair it's not an actual problem) is that we might truncate a long sockpath without any warning or error. I don't have a particularly good alternative suggestion. Maybe using strlen + an assert? This is why we need Jim Meyering to come back :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org