From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: [Patch v2 3/4] tools/xenstored: Avoid buffer overflows while setting up sockets Date: Mon, 25 Nov 2013 14:38:41 +0000 Message-ID: <1385390321-10124-1-git-send-email-andrew.cooper3@citrix.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Xen-devel Cc: Andrew Cooper , Matthew Daley , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org Coverity ID: 1055996 1056002 Cache the xs_daemon_socket{,_ro}() strings to save pointlessly re-snprintf()'ing the same path, and add explicit size checks against addr.sun_path before strcpy()'ing into it. Signed-off-by: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: Matthew Daley --- Changes in v2: * Use logic similar to f220279c14 --- tools/xenstore/xenstored_core.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index ccfdaa3..2324e53 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -1718,6 +1718,9 @@ static void init_sockets(int **psock, int **pro_sock) { struct sockaddr_un addr; int *sock, *ro_sock; + const char *soc_str = xs_daemon_socket(); + const char *soc_str_ro = xs_daemon_socket_ro(); + /* Create sockets for them to listen to. */ *psock = sock = talloc(talloc_autofree_context(), int); *sock = socket(PF_UNIX, SOCK_STREAM, 0); @@ -1731,19 +1734,25 @@ static void init_sockets(int **psock, int **pro_sock) talloc_set_destructor(ro_sock, destroy_fd); /* FIXME: Be more sophisticated, don't mug running daemon. */ - unlink(xs_daemon_socket()); - unlink(xs_daemon_socket_ro()); + unlink(soc_str); + unlink(soc_str_ro); addr.sun_family = AF_UNIX; - strcpy(addr.sun_path, xs_daemon_socket()); + + if(strlen(soc_str) >= sizeof(addr.sun_path)) + barf_perror("socket string '%s' too long", soc_str); + strcpy(addr.sun_path, soc_str); 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", soc_str); + + if(strlen(soc_str_ro) >= sizeof(addr.sun_path)) + barf_perror("socket string '%s' too long", soc_str_ro); + strcpy(addr.sun_path, soc_str_ro); if (bind(*ro_sock, (struct sockaddr *)&addr, sizeof(addr)) != 0) - barf_perror("Could not bind socket to %s", - xs_daemon_socket_ro()); - if (chmod(xs_daemon_socket(), 0600) != 0 - || chmod(xs_daemon_socket_ro(), 0660) != 0) + barf_perror("Could not bind socket to %s", soc_str_ro); + + if (chmod(soc_str, 0600) != 0 + || chmod(soc_str_ro, 0660) != 0) barf_perror("Could not chmod sockets"); if (listen(*sock, 1) != 0 -- 1.7.10.4