'virStoragePoolObjListSearch' returns a locked and refed object, thus we
must release it on ACL permission failure.

Fixes: 7aa0e8c0cb8
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1984318
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
Technically a security issue since it DoS-es the objects a user doesn't
have access to for users which do have access.

Posting to standard devel list since the bugzilla above is public.

 src/storage/storage_driver.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index c2ff4b8d06..6aa10d89f6 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1738,8 +1738,10 @@ storagePoolLookupByTargetPath(virConnectPtr conn,
                                            storagePoolLookupByTargetPathCallback,
                                            cleanpath))) {
         def = virStoragePoolObjGetDef(obj);
-        if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0)
+        if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0) {
+            virStoragePoolObjEndAPI(&obj);
             return NULL;
+        }

         pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
         virStoragePoolObjEndAPI(&obj);
-- 
2.31.1

Adding libvirt-security since I forgot when sending the patch.

On Wed, Jul 21, 2021 at 11:27:41 +0200, Peter Krempa wrote:
> 'virStoragePoolObjListSearch' returns a locked and refed object, thus we
> must release it on ACL permission failure.
> 
> Fixes: 7aa0e8c0cb8
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1984318
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> Technically a security issue since it DoS-es the objects a user doesn't
> have access to for users which do have access.
> 
> Posting to standard devel list since the bugzilla above is public.
> 
>  src/storage/storage_driver.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index c2ff4b8d06..6aa10d89f6 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -1738,8 +1738,10 @@ storagePoolLookupByTargetPath(virConnectPtr conn,
>                                             storagePoolLookupByTargetPathCallback,
>                                             cleanpath))) {
>          def = virStoragePoolObjGetDef(obj);
> -        if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0)
> +        if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0) {
> +            virStoragePoolObjEndAPI(&obj);
>              return NULL;
> +        }
> 
>          pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
>          virStoragePoolObjEndAPI(&obj);
> -- 
> 2.31.1
> 

Hi Peter,

I'm going to allocate a new (low impact) CVE for this bug.

Thanks.

On Wed, Jul 21, 2021 at 11:34 AM Peter Krempa <pkrempa@redhat.com> wrote:
>
> Adding libvirt-security since I forgot when sending the patch.
>
> On Wed, Jul 21, 2021 at 11:27:41 +0200, Peter Krempa wrote:
> > 'virStoragePoolObjListSearch' returns a locked and refed object, thus we
> > must release it on ACL permission failure.
> >
> > Fixes: 7aa0e8c0cb8
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1984318
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> > Technically a security issue since it DoS-es the objects a user doesn't
> > have access to for users which do have access.
> >
> > Posting to standard devel list since the bugzilla above is public.
> >
> >  src/storage/storage_driver.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> > index c2ff4b8d06..6aa10d89f6 100644
> > --- a/src/storage/storage_driver.c
> > +++ b/src/storage/storage_driver.c
> > @@ -1738,8 +1738,10 @@ storagePoolLookupByTargetPath(virConnectPtr conn,
> >                                             storagePoolLookupByTargetPathCallback,
> >                                             cleanpath))) {
> >          def = virStoragePoolObjGetDef(obj);
> > -        if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0)
> > +        if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0) {
> > +            virStoragePoolObjEndAPI(&obj);
> >              return NULL;
> > +        }
> >
> >          pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
> >          virStoragePoolObjEndAPI(&obj);
> > --
> > 2.31.1
> >
>


-- 
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0

On Thu, Jul 22, 2021 at 11:14 AM Mauro Matteo Cascella
<mcascell@redhat.com> wrote:
>
> Hi Peter,
>
> I'm going to allocate a new (low impact) CVE for this bug.

This issue was assigned CVE-2021-3667.

> Thanks.

-- 
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0

On 7/21/21 11:27 AM, Peter Krempa wrote:
> 'virStoragePoolObjListSearch' returns a locked and refed object, thus we
> must release it on ACL permission failure.
> 
> Fixes: 7aa0e8c0cb8
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1984318
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> Technically a security issue since it DoS-es the objects a user doesn't
> have access to for users which do have access.
> 
> Posting to standard devel list since the bugzilla above is public.
> 
>  src/storage/storage_driver.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal