On Tue, Mar 09, 2021 at 03:26:22PM +0100, Michal Privoznik wrote: > The qemu_shim (compiled into virt-qemu-run-binary) reads several > files provided by user (XML definition of secret, value of the > secret, XML definition of domain) and it does so using > g_file_get_contents(). This is potentially dangerous, because > there is no limit on the size of files/buffers. > > Since this is a standalone binary it's not critical as it can't > cause libvirtd to be OOM killed, but it's still worth fixing as I > am planning on discouraging people from using the GLib function. I'm not convinced actually. I think that almost all of the places where we use virFileReadAll with a limit are in fact safe to use with g_file_get_contents. Almost all the files are from trusted sources - files exposed by the kernel, or files only writable by root, or by the same user as libvirtd. Almost no where does libvirt read files that are provided by an untrusted user. Reading disk image headers, or QEMU save files are the two main untrusted scenarios, and in both those cases we wdon't actually want more than the header. I think we probably get rid of virFileReadAll in the long term Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
