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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel