On Wed, Apr 17, 2024 at 02:03:38PM +0200, Ján Tomko wrote:
On a Tuesday in 2024, Daniel P. Berrangé wrote:
> On Tue, Apr 16, 2024 at 12:58:53PM +0200, Ján Tomko wrote:
> > On a Tuesday in 2024, Daniel P. Berrangé wrote:
> > > The typed parameter array must be either 0, or a positive
> > > number.
> > >
> >
> > Does this matter?
> >
> > The API documentation says:
> > * @nparams: pointer to received number of interface parameter
> >
> > and it looks like we ignore the number as long as params is NULL.
>
> This missing check is something I noticed when fixing the recent
> CVE about RPC checking nparams. In all other APIs we have such
> a virCheckNonNegativeArgGoto for '*nparams', this was the only
> one that was missing.
>
> I believe it is harmless in terms of risk to libvirt/libvirtd,
> but it might lead to better detection/reporting of bugs in apps.
>
I was thinking someone initializing *nparams to -1 could be a valid use
according to the documentation (with params = NULL), but if we do it the
same for all other similar APIs, it's okay.
However, it seems 'nparams' being NULL is not valid whether it's
the user or libvirt allocating the array, so please also add:
virCheckNonNullArgGoto(nparams, error);
Opps yes, all the other APIs have this check, so should have
it here too.
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
With 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 :|