Re: [PATCH for-9.1 0/2] NBD: don't print raw server error text to terminal

["Richard W.M. Jones" <rjones@redhat.com>]

On Mon, Aug 05, 2024 at 01:48:12PM -0500, Eric Blake wrote:
> On Fri, Aug 02, 2024 at 02:26:04PM GMT, Eric Blake wrote:
> > I've requested a CVE from Red Hat, and hope to have an assigned number
> > soon.  Meanwhile, we can get review started, to make sure this is
> > ready to include in 9.1.  'qemu-img info' should never print untrusted
> > data in a way that might take over a user's terminal.
> > 
> > There are probably other spots where qemu-img info is printing
> > untrusted data (such as filenames), where we probably ought to use the
> > same sanitization tactics as shown here.  Identifying those spots
> > would be a useful part of this review, and may mean a v2 that is even
> > more extensive in the number of patches.
> 
> In fact, should we insist that 'qemu-img info XXX' refuse to accept
> any control characters on any command-line filename, and that it
> reject any backing file name with control characters as a malformed
> qcow2 file?  For reference, we JUST fixed qemu-img info to change
> qcow2 files with a claimed backing file of json:... to favor the local
> file ./json:... over the potentially dangerous user-controlled
> format/protocol description, so we are _already_ changing how strict
> qemu-img is for 9.1, and adding one more restriction to avoid
> escape-sequence madness makes sense.
> 
> Note that with:
> 
> touch $'\e[m' && qemu-img info --output=json $'\e[m'
> 
> we already escape our output, but without --output=json, we are
> vulnerable to user-controlled ESC leaking through to stdout for more
> than just the NBD server errors that I addressed in v1 of this patch
> series.  Hence my question on whether v2 of the series should touch
> more places in the code, and whether doing something like flat-out
> refusing users stupid enough to embed control characters in their
> filenames is a safe change this close to release.

You could say if someone gives you a "malicious" text file which you
cat to stdout, it could change your terminal settings.  I don't think
therefore any of this is very serious.  We should probably fix any
obvious things, but it doesn't need to happen right before 9.1 is
released, we can take our time.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top