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@xxxxxxxxxx> > 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 :| _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
