[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [RFC][PATCH v5 08/21] virtagent: add agent_viewfile qmp

From: Michael Roth
Subject: [Qemu-devel] Re: [RFC][PATCH v5 08/21] virtagent: add agent_viewfile qmp/hmp command
Date: Fri, 10 Dec 2010 11:09:48 -0600
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv: Gecko/20101027 Thunderbird/3.1.6

On 12/10/2010 12:43 AM, Jes Sorensen wrote:
On 12/09/10 22:12, Michael Roth wrote:
On 12/07/2010 08:26 AM, Jes Sorensen wrote:
I believe this suffers from the same architectural problem I mentioned
in my comment to 07/21 - you don't restrict the file size, so it could
blow up the QEMU process on the host trying to view the wrong file.

It's restricted on the guest side:


     while ((ret = read(fd, buf, VA_FILEBUF_LEN))>  0) {
         file_contents = qemu_realloc(file_contents, count +
         memcpy(file_contents + count, buf, ret);
         count += ret;
         if (count>  VA_GETFILE_MAX) {
             xmlrpc_faultf(env, "max file size (%d bytes) exceeded",
             goto EXIT_CLOSE_BAD;

You cannot rely on the guest controlling this. You really have to treat
any guest as hostile and keep control and security in the host,
otherwise a hacked guest could end up attacking the host by blowing up
the host's QEMU process.

Definetely agree on this, I mentioned some other checks here at the transport and host xmlrpc level that would also limit this possibility:

> There are additional limits at the transport layer well to deal with a
> potentially malicious/buggy agent as well:
> virtagent-common.c:va_http_read_handler():
>              } else if (s->content_len > VA_CONTENT_LEN_MAX) {
>                  LOG("http content length too long");
>                  goto out_bad;
>              }

I think with strictly enforced size limits the major liability for viewfile is, as you mentioned, users using it to view binary data or carefully crafted files that can mess up or fool users/shells/programs interpreting monitor output.

But plain-text does not include escape sequences, so it's completely reasonable that we'd scrape them. And I'm not sure if a "(qemu)" in the text is a potential liability. Would there be any other issues to consider?

If we can guard against those things, do you agree it wouldn't be an inherently dangerous interface? State-full, asynchronous RPCs like copyfile and exec are not really something I'd planned for the initial release. I think they'll take some time to get right, and a simple low-risk interface to cover what I'm fairly sure is the most common use case seems reasonable.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]