On Thu, May 11, 2023 at 09:10:32PM -0500, Eric Blake wrote: > > Previous patches changed all integral qemu_strto*() error paths to > guarantee that *value is never left uninitialized. Do likewise for > qemu_strtod. Also, tighten qemu_strtod_finite() to never return a > non-finite value (prior to this patch, we were rejecting "inf" with > -EINVAL and unspecified result 0.0, but failing "9e999" with -ERANGE > and HUGE_VAL - which is infinite on IEEE machines - despite our > function claiming to recognize only finite values). > > Auditing callers, we have no external callers of qemu_strtod, and > among the callers of qemu_strtod_finite: > > - qapi/qobject-input-visitor.c:qobject_input_type_number_keyval() and > qapi/string-input-visitor.c:parse_type_number() which reject all > errors (does not matter what we store) > > - utils/cutils.c:do_strtosz() incorrectly assumes that *endptr points > to '.' on all failures (that is, it is not distinguishing between > EINVAL and ERANGE; and therefore still does the WRONG THING for > "9.9e999". The change here does not entirely fix that (a later > patch will tackle this more systematically), but at least the value > of endptr is now less likely to be out of bounds on overflow > > - our testsuite, which we can update to match what we document > > Signed-off-by: Eric Blake <ebl...@redhat.com> > Reviewed-by: Hanna Czenczek <hre...@redhat.com> > > ---
cc'ing qemu-stable for comments, since this patch fixes a sanitizer issue flagged in https://gitlab.com/qemu-project/qemu/-/issues/1629. While we decided the read-out-of-bounds in qemu_strtosz() is probably not CVE-worthy (if you have control over the command line or QMP, you can probably do much worse than cause a size parser to segfault), it does raise the question of whether this patch is useful for qemu-stable. What's more, taking this patch in isolation without all the prerequisite patches is probably sufficient to prevent the read-out-of-bounds, but still leaves qemu_strtosz() with some odd corner cases; while taking the whole series is a much bigger committment but easier to analyze as a unit given that a lot of thte series is testsuite unit test additions. But either way, we'd need reviews on this series if anyone thinks it warrants backports to stable releases. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
