[PATCH] libsepol: destroy filename_trans list properly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



OSS-Fuzz found a direct memory leak in policydb_filetrans_insert()
because filenametr_destroy() does not fully destroy the list associated
with a typetransition.

More precisely, let's consider this (minimized) CIL policy:

    (class CLASS (PERM))
    (classorder (CLASS))
    (sid SID)
    (sidorder (SID))
    (user USER)
    (role ROLE)
    (type TYPE) ; "type 1" in libsepol internal structures
    (type TYPE2) ; "type 2" in libsepol internal structures
    (type TYPE3) ; "type 3" in libsepol internal structures
    (category CAT)
    (categoryorder (CAT))
    (sensitivity SENS)
    (sensitivityorder (SENS))
    (sensitivitycategory SENS (CAT))
    (allow TYPE self (CLASS (PERM)))
    (roletype ROLE TYPE)
    (userrole USER ROLE)
    (userlevel USER (SENS))
    (userrange USER ((SENS)(SENS (CAT))))
    (sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))

    (typetransition TYPE2 TYPE CLASS "some_file" TYPE2)
    (typetransition TYPE3 TYPE CLASS "some_file" TYPE3)

The two typetransition statements make policydb_filetrans_insert()
insert an item with key {ttype=1, tclass=1, name="some_file"} in the
hashmap p->filename_trans. This item contains a linked list of two
filename_trans_datum_t elements:

* The first one uses {otype=2, stypes=bitmap containing 2}
* The second one uses {otype=3, stypes=bitmap containing 3}

Nevertheless filenametr_destroy() (called by
hashtab_map(p->filename_trans, filenametr_destroy, NULL);) only frees
the first element. Fix this memory leak by freeing all elements.

This issue was introduced by commit 42ae834a7428 ("libsepol,checkpolicy:
optimize storage of filename transitions") and was never present in the
kernel, as filenametr_destroy() was modified appropriately in commit
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c3a276111ea2572399281988b3129683e2a6b60b

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29138
Signed-off-by: Nicolas Iooss <nicolas.iooss@xxxxxxx>
---
 libsepol/src/policydb.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index f43d5c67463e..71ada42ca609 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -1462,12 +1462,16 @@ static int filenametr_destroy(hashtab_key_t key, hashtab_datum_t datum,
 			      void *p __attribute__ ((unused)))
 {
 	filename_trans_key_t *ft = (filename_trans_key_t *)key;
-	filename_trans_datum_t *fd = datum;
+	filename_trans_datum_t *fd = datum, *next;
 
 	free(ft->name);
 	free(key);
-	ebitmap_destroy(&fd->stypes);
-	free(datum);
+	do {
+		next = fd->next;
+		ebitmap_destroy(&fd->stypes);
+		free(fd);
+		fd = next;
+	} while (fd);
 	return 0;
 }
 
-- 
2.30.0




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux