On Thu, Mar 14, 2019 at 12:16:06PM +0000, Peter Maydell wrote: > On Thu, 14 Mar 2019 at 11:23, Paolo Bonzini <pbonz...@redhat.com> wrote: > > > > On 14/03/19 11:51, Peter Maydell wrote: > > > Our coverity model of g_strdup() includes: > > > __coverity_string_size_sink__(s); > > > > > > This seems to be causing Coverity to report false positives like > > > CID1399705 and 1399699 where we take a string from getenv() and > > > pass it to g_strdup() The getenv() string is untrusted data of unknown > > > length, and g_strdup() being marked as a size-sink makes Coverity > > > think the function wants "a string of a particular size". > > > > > > Markus, you wrote this model initially -- can you remember why it's > > > marked as a size-sink? Unfortunately I can't find any documentation > > > online about what the coverity model annotation here means :-( > > > > I think it means that we don't want a g_strdup that can potentially do > > an unbounded allocation. > > Mmm, that makes sense. So in this particular case, do we > want to try to avoid doing an unbounded allocation based > on whatever rubbish the user passed us in the environment, > or do we say "this particular case is OK" and mark it > as a false-positive ?
I'd mark it a false positive. QEMU trusts whatever parent process has spawned it to provide required environment variables, so it should honour whatever env is set. ie the mgmt app above QEMU is not an attack vector QEMU protects itself against. 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 :|
