| Message ID | 1509121935-41889-1-git-send-email-wipawel@amazon.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Pawel Wieczorkiewicz writes ("[PATCH] tools/xenstored: Check number of strings passed to do_control()"):
> It is possible to send a zero-string message body to xenstore's
> XS_CONTROL handling function. Then the number of strings is used
> for an array allocation. This leads to a crash in strcmp() in a
> CONTROL sub-command invocation loop.
> The output of xs_count_string() should be verified and all 0 or
> negative values should be rejected with an EINVAL. At least the
> sub-command name must be specified.
>
> The xenstore crash can only be triggered from within dom0 (there
> is a check in do_control() rejecting all non-dom0 requests with
> an EACCES).
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(Added the for-4.10 tag to the Subject.)
Ian.
On 27/10/17 18:32, Pawel Wieczorkiewicz wrote: > It is possible to send a zero-string message body to xenstore's > XS_CONTROL handling function. Then the number of strings is used > for an array allocation. This leads to a crash in strcmp() in a > CONTROL sub-command invocation loop. > The output of xs_count_string() should be verified and all 0 or > negative values should be rejected with an EINVAL. At least the > sub-command name must be specified. > > The xenstore crash can only be triggered from within dom0 (there > is a check in do_control() rejecting all non-dom0 requests with > an EACCES). > > Testing: reproduced with the following command: > python -c 'print 16*"\x00"' | nc -U $XENSTORED_RUNDIR/socket > > Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de> > Reviewed-by: Martin Pohlack <mpohlack@amazon.de> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
On Fri, Oct 27, 2017 at 04:32:15PM +0000, Pawel Wieczorkiewicz wrote: > It is possible to send a zero-string message body to xenstore's > XS_CONTROL handling function. Then the number of strings is used > for an array allocation. This leads to a crash in strcmp() in a > CONTROL sub-command invocation loop. > The output of xs_count_string() should be verified and all 0 or > negative values should be rejected with an EINVAL. At least the > sub-command name must be specified. > > The xenstore crash can only be triggered from within dom0 (there > is a check in do_control() rejecting all non-dom0 requests with > an EACCES). > > Testing: reproduced with the following command: > python -c 'print 16*"\x00"' | nc -U $XENSTORED_RUNDIR/socket > > Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de> > Reviewed-by: Martin Pohlack <mpohlack@amazon.de> Acked-by: Wei Liu <wei.liu2@citrix.com>
Hi, Apologies for the late answer, I missed the e-mail in my inbox. On 10/27/2017 05:37 PM, Ian Jackson wrote: > Pawel Wieczorkiewicz writes ("[PATCH] tools/xenstored: Check number of strings passed to do_control()"): >> It is possible to send a zero-string message body to xenstore's >> XS_CONTROL handling function. Then the number of strings is used >> for an array allocation. This leads to a crash in strcmp() in a >> CONTROL sub-command invocation loop. >> The output of xs_count_string() should be verified and all 0 or >> negative values should be rejected with an EINVAL. At least the >> sub-command name must be specified. >> >> The xenstore crash can only be triggered from within dom0 (there >> is a check in do_control() rejecting all non-dom0 requests with >> an EACCES). > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > (Added the for-4.10 tag to the Subject.) Release-acked-by: Julien Grall <julien.grall@linaro.org> Cheers,
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c index 7c14911..e4b8aa9 100644 --- a/tools/xenstore/xenstored_control.c +++ b/tools/xenstore/xenstored_control.c @@ -184,6 +184,8 @@ int do_control(struct connection *conn, struct buffered_data *in) return EACCES; num = xs_count_strings(in->buffer, in->used); + if (num < 1) + return EINVAL; vec = talloc_array(in, char *, num); if (!vec) return ENOMEM;