On Mon, Aug 05, 2024 at 08:11:31PM GMT, Richard W.M. Jones wrote: > 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.
After consulting with Red Hat security, they agree: their decision at this time is that any CVE related to escape sequences taking over a terminal would best be filed against the terminal and/or shell that allowed the escape, rather than against every single app that can produce such pass-through output. So at this time they were unwilling to issue a CVE against qemu for this particular issue, and I will clean up the subject line for v2. So I agree that cleaning up low-hanging fruit is still worth it, but no longer a reason to rush this series into 9.1. If it still gets a timely positive review, I can include it alongside the other NBD patches (we are fixing a CVE with qemu hitting SEGV on nbd-server-stop). -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
