5:13 a.m.
The typed parameter array must be either 0, or a positive
number.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libvirt-network.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index ef17a8a04d..e467716b6a 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -1577,6 +1577,8 @@ virNetworkPortGetParameters(virNetworkPortPtr port,
virCheckNetworkPortReturn(port, -1);
conn = port->net->conn;
+ virCheckNonNegativeArgGoto(*nparams, error);
+
if (conn->networkDriver &&
conn->networkDriver->networkPortGetParameters) {
int ret;
ret = conn->networkDriver->networkPortGetParameters(port, params, nparams,
flags);
--
2.43.0
5:58 a.m.
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.
Jano
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libvirt-network.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index ef17a8a04d..e467716b6a 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -1577,6 +1577,8 @@ virNetworkPortGetParameters(virNetworkPortPtr port,
virCheckNetworkPortReturn(port, -1);
conn = port->net->conn;
+ virCheckNonNegativeArgGoto(*nparams, error);
+
if (conn->networkDriver &&
conn->networkDriver->networkPortGetParameters) {
int ret;
ret = conn->networkDriver->networkPortGetParameters(port, params, nparams,
flags);
--
2.43.0
_______________________________________________
Devel mailing list -- devel(a)lists.libvirt.org
To unsubscribe send an email to devel-leave(a)lists.libvirt.org
6:10 a.m.
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.
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/libvirt-network.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/src/libvirt-network.c b/src/libvirt-network.c
> index ef17a8a04d..e467716b6a 100644
> --- a/src/libvirt-network.c
> +++ b/src/libvirt-network.c
> @@ -1577,6 +1577,8 @@ virNetworkPortGetParameters(virNetworkPortPtr port,
> virCheckNetworkPortReturn(port, -1);
> conn = port->net->conn;
>
> + virCheckNonNegativeArgGoto(*nparams, error);
> +
> if (conn->networkDriver &&
conn->networkDriver->networkPortGetParameters) {
> int ret;
> ret = conn->networkDriver->networkPortGetParameters(port, params,
nparams, flags);
> --
> 2.43.0
> _______________________________________________
> Devel mailing list -- devel(a)lists.libvirt.org
> To unsubscribe send an email to devel-leave(a)lists.libvirt.org
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 :|
7:03 a.m.
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);
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
> > Signed-off-by: Daniel P. Berrangé
<berrange(a)redhat.com>
> > ---
> > src/libvirt-network.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/src/libvirt-network.c b/src/libvirt-network.c
> > index ef17a8a04d..e467716b6a 100644
> > --- a/src/libvirt-network.c
> > +++ b/src/libvirt-network.c
> > @@ -1577,6 +1577,8 @@ virNetworkPortGetParameters(virNetworkPortPtr port,
> > virCheckNetworkPortReturn(port, -1);
> > conn = port->net->conn;
> >
> > + virCheckNonNegativeArgGoto(*nparams, error);
> > +
> > if (conn->networkDriver &&
conn->networkDriver->networkPortGetParameters) {
> > int ret;
> > ret = conn->networkDriver->networkPortGetParameters(port, params,
nparams, flags);
> > --
> > 2.43.0
> > _______________________________________________
> > Devel mailing list -- devel(a)lists.libvirt.org
> > To unsubscribe send an email to devel-leave(a)lists.libvirt.org
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 :|
7:12 a.m.
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 :|
250
days inactive
251
days old
4 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Ján Tomko