[PATCH] libxl: Fix format string abuses / vulnerabilities

[PATCH] libxl: Fix format string abuses / vulnerabilities

[Ian Jackson]

There are a few places where libxl__xs_write is passed a variable
value to write to xenstore, but the semantics are that the first char*
is a format string.  So use "%s".

This fixes the following errors reported by some newer compilers:
 libxl.c: In function "libxl_create_cpupool":
 libxl.c:3981: error: format not a string literal and no format arguments
 libxl.c:3983: error: format not a string literal and no format arguments
 libxl.c: In function "libxl_cpupool_movedomain":
 libxl.c:4095: error: format not a string literal and no format arguments

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff -r 28a160746815 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Wed Oct 27 12:24:28 2010 +0100
+++ b/tools/libxl/libxl.c	Wed Oct 27 13:38:30 2010 +0100
@@ -3977,10 +3977,12 @@ int libxl_create_cpupool(libxl_ctx *ctx,
         t = xs_transaction_start(ctx->xsh);
 
         xs_mkdir(ctx->xsh, t, libxl__sprintf(&gc, "/local/pool/%d", *poolid));
-        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "/local/pool/%d/uuid", *poolid),
-                 uuid_string);
-        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "/local/pool/%d/name", *poolid),
-                 name);
+        libxl__xs_write(&gc, t,
+                        libxl__sprintf(&gc, "/local/pool/%d/uuid", *poolid),
+                        "%s", uuid_string);
+        libxl__xs_write(&gc, t,
+                        libxl__sprintf(&gc, "/local/pool/%d/name", *poolid),
+                        "%s", name);
 
         if (xs_transaction_end(ctx->xsh, t, 0) || (errno != EAGAIN))
             return 0;
@@ -4092,7 +4094,8 @@ int libxl_cpupool_movedomain(libxl_ctx *
         if (!vm_path)
             break;
 
-        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "%s/pool_name", vm_path), poolname);
+        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "%s/pool_name", vm_path),
+                        "%s", poolname);
 
         if (xs_transaction_end(ctx->xsh, t, 0) || (errno != EAGAIN))
             break;

Re: [PATCH] libxl: Fix format string abuses / vulnerabilities

[Ian Campbell]

On Wed, 2010-10-27 at 14:17 +0100, Ian Jackson wrote:
> There are a few places where libxl__xs_write is passed a variable
> value to write to xenstore, but the semantics are that the first char*
> is a format string.  So use "%s".
> 
> This fixes the following errors reported by some newer compilers:
>  libxl.c: In function "libxl_create_cpupool":
>  libxl.c:3981: error: format not a string literal and no format arguments
>  libxl.c:3983: error: format not a string literal and no format arguments
>  libxl.c: In function "libxl_cpupool_movedomain":
>  libxl.c:4095: error: format not a string literal and no format arguments
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Makes sense.

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> diff -r 28a160746815 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Wed Oct 27 12:24:28 2010 +0100
> +++ b/tools/libxl/libxl.c	Wed Oct 27 13:38:30 2010 +0100
> @@ -3977,10 +3977,12 @@ int libxl_create_cpupool(libxl_ctx *ctx,
>          t = xs_transaction_start(ctx->xsh);
>  
>          xs_mkdir(ctx->xsh, t, libxl__sprintf(&gc, "/local/pool/%d", *poolid));
> -        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "/local/pool/%d/uuid", *poolid),
> -                 uuid_string);
> -        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "/local/pool/%d/name", *poolid),
> -                 name);
> +        libxl__xs_write(&gc, t,
> +                        libxl__sprintf(&gc, "/local/pool/%d/uuid", *poolid),
> +                        "%s", uuid_string);
> +        libxl__xs_write(&gc, t,
> +                        libxl__sprintf(&gc, "/local/pool/%d/name", *poolid),
> +                        "%s", name);
>  
>          if (xs_transaction_end(ctx->xsh, t, 0) || (errno != EAGAIN))
>              return 0;
> @@ -4092,7 +4094,8 @@ int libxl_cpupool_movedomain(libxl_ctx *
>          if (!vm_path)
>              break;
>  
> -        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "%s/pool_name", vm_path), poolname);
> +        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "%s/pool_name", vm_path),
> +                        "%s", poolname);
>  
>          if (xs_transaction_end(ctx->xsh, t, 0) || (errno != EAGAIN))
>              break;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] libxl: Fix format string abuses / vulnerabilities

[Gianni Tedesco]

On Wed, 2010-10-27 at 14:17 +0100, Ian Jackson wrote:
> There are a few places where libxl__xs_write is passed a variable
> value to write to xenstore, but the semantics are that the first char*
> is a format string.  So use "%s".
> 
> This fixes the following errors reported by some newer compilers:
>  libxl.c: In function "libxl_create_cpupool":
>  libxl.c:3981: error: format not a string literal and no format arguments
>  libxl.c:3983: error: format not a string literal and no format arguments
>  libxl.c: In function "libxl_cpupool_movedomain":
>  libxl.c:4095: error: format not a string literal and no format arguments
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> diff -r 28a160746815 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Wed Oct 27 12:24:28 2010 +0100
> +++ b/tools/libxl/libxl.c	Wed Oct 27 13:38:30 2010 +0100
> @@ -3977,10 +3977,12 @@ int libxl_create_cpupool(libxl_ctx *ctx,
>          t = xs_transaction_start(ctx->xsh);
>  
>          xs_mkdir(ctx->xsh, t, libxl__sprintf(&gc, "/local/pool/%d", *poolid));
> -        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "/local/pool/%d/uuid", *poolid),
> -                 uuid_string);
> -        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "/local/pool/%d/name", *poolid),
> -                 name);
> +        libxl__xs_write(&gc, t,
> +                        libxl__sprintf(&gc, "/local/pool/%d/uuid", *poolid),
> +                        "%s", uuid_string);
> +        libxl__xs_write(&gc, t,
> +                        libxl__sprintf(&gc, "/local/pool/%d/name", *poolid),
> +                        "%s", name);
>  
>          if (xs_transaction_end(ctx->xsh, t, 0) || (errno != EAGAIN))
>              return 0;
> @@ -4092,7 +4094,8 @@ int libxl_cpupool_movedomain(libxl_ctx *
>          if (!vm_path)
>              break;
>  
> -        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "%s/pool_name", vm_path), poolname);
> +        libxl__xs_write(&gc, t, libxl__sprintf(&gc, "%s/pool_name", vm_path),
> +                        "%s", poolname);
>  

Good lord!

Ack.

Gianni

Re: [PATCH] libxl: Fix format string abuses / vulnerabilities [and 2 more messages]

[Ian Jackson]

Ian Jackson writes ("[Xen-devel] [PATCH] libxl: Fix format string abuses / vulnerabilities"):
> There are a few places where libxl__xs_write is passed a variable
> value to write to xenstore, but the semantics are that the first char*
> is a format string.  So use "%s".

Ian Campbell writes ("Re: [Xen-devel] [PATCH] libxl: Fix format string abuses / vulnerabilities"):
> Makes sense.

Gianni Tedesco writes ("Re: [Xen-devel] [PATCH] libxl: Fix format string abuses / vulnerabilities"):
> Good lord!
> Ack.

I've applied it.

Thanks,
Ian.