From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthew Daley Subject: Re: [PATCH 3/4] tools/xenstored: Avoid buffer overflows while setting up sockets Date: Tue, 26 Nov 2013 01:25:29 +1300 Message-ID: References: <1385377664-20979-1-git-send-email-andrew.cooper3@citrix.com> <1385377664-20979-4-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1385377664-20979-4-git-send-email-andrew.cooper3@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: Ian Jackson , Ian Campbell , Xen-devel List-Id: xen-devel@lists.xenproject.org On Tue, Nov 26, 2013 at 12:07 AM, Andrew Cooper wrote: > Coverity ID: 1055996 1056002 > > Use strncpy in preference to strcpy, and use the correct failing path for > error messages. > > Signed-off-by: Andrew Cooper > CC: Ian Campbell > CC: Ian Jackson > --- > tools/xenstore/xenstored_core.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c > index ccfdaa3..3c13c64 100644 > --- a/tools/xenstore/xenstored_core.c > +++ b/tools/xenstore/xenstored_core.c > @@ -1735,13 +1735,12 @@ static void init_sockets(int **psock, int **pro_sock) > unlink(xs_daemon_socket_ro()); > > addr.sun_family = AF_UNIX; > - strcpy(addr.sun_path, xs_daemon_socket()); > + strncpy(addr.sun_path, xs_daemon_socket(), sizeof(addr.sun_path)); Nitpick: Using strncpy in this manner is unsafe as it does not guarantee the null-termination of the result in the destination buffer (which would presumably make barf_perror unhappy). For that reason, and to avoid truncation, I check the source string length directly instead in commit f220279c14, for example. - Matthew > if (bind(*sock, (struct sockaddr *)&addr, sizeof(addr)) != 0) > - barf_perror("Could not bind socket to %s", xs_daemon_socket()); > - strcpy(addr.sun_path, xs_daemon_socket_ro()); > + barf_perror("Could not bind socket to %s", addr.sun_path); > + strncpy(addr.sun_path, xs_daemon_socket_ro(), sizeof(addr.sun_path)); > if (bind(*ro_sock, (struct sockaddr *)&addr, sizeof(addr)) != 0) > - barf_perror("Could not bind socket to %s", > - xs_daemon_socket_ro()); > + barf_perror("Could not bind socket to %s", addr.sun_path); > if (chmod(xs_daemon_socket(), 0600) != 0 > || chmod(xs_daemon_socket_ro(), 0660) != 0) > barf_perror("Could not chmod sockets"); > -- > 1.7.10.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel