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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
