Re: [PATCH 3/4] tools/xenstored: Avoid buffer overflows while setting up sockets

[Ian Jackson <Ian.Jackson@eu.citrix.com>]

Andrew Cooper writes ("[PATCH 3/4] tools/xenstored: Avoid buffer overflows while setting up sockets"):
> Coverity ID: 1055996 1056002
> 
> Use strncpy in preference to strcpy, and use the correct failing path for
> error messages.
...
>  	addr.sun_family = AF_UNIX;
> -	strcpy(addr.sun_path, xs_daemon_socket());
> +	strncpy(addr.sun_path, xs_daemon_socket(), sizeof(addr.sun_path));
>  	if (bind(*sock, (struct sockaddr *)&addr, sizeof(addr)) != 0)
> -		barf_perror("Could not bind socket to %s", xs_daemon_socket());
> +		barf_perror("Could not bind socket to %s", addr.sun_path);

This latter hunk is not correct.  addr.sun_path might not be
nul-terminated.  xs_daemon_socket() is, but isn't the path actually
attempted.

Also, while this new code avoids UB, it still has the bug that if the
configured socket pathname is too long, xenstored will create a
version with a truncated path.

Perhaps a better approach would be an explicit overlength check.

Ian.