Re: [Xen-devel] [PATCH] libxl: make sure string is null-terminated in libxl__prepare_sockaddr_un

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Wei Liu writes ("[PATCH] libxl: make sure string is null-terminated in 
libxl__prepare_sockaddr_un"):
> Coverity-ID: 1438472
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>

But...

> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index 5854717b11..e06f765699 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -1238,6 +1238,8 @@ int libxl__prepare_sockaddr_un(libxl__gc *gc,
>                                 struct sockaddr_un *un, const char *path,
>                                 const char *what)
>  {
> +    size_t sz = sizeof(un->sun_path);
> +
>      if (sizeof(un->sun_path) <= strlen(path)) {
>          LOG(ERROR, "UNIX socket path '%s' is too long for %s", path, what);
>          LOG(DEBUG, "Path must be less than %zu bytes", sizeof(un->sun_path));
           return ERROR_INVAL;
> @@ -1245,7 +1247,8 @@ int libxl__prepare_sockaddr_un(libxl__gc *gc,
>      }
>      memset(un, 0, sizeof(struct sockaddr_un));
>      un->sun_family = AF_UNIX;
> -    strncpy(un->sun_path, path, sizeof(un->sun_path));
> +    strncpy(un->sun_path, path, sz);
> +    un->sun_path[sz - 1] = '\0';

If we have reached this point, sizeof(un->sun_path) > strlen(path).
So in fact, strcpy would do.  strncpy will always add a nul.

We just memset the whole struct to 0.

If this new code has any effect at all, it will corrupt the string by
truncating it.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel