On Mon, Jan 22, 2024 at 9:02 AM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> wrote:
>
> Add a new member to the struct level_datum to indicate whether the
> member `level` is owned by the current instance, and free it on cleanup
> only then.
>
> This helps to implement a fix for a use-after-free issue in the
> checkpolicy(8) compiler.
>
> Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> ---
> libsepol/include/sepol/policydb/policydb.h | 1 +
> libsepol/src/policydb.c | 6 ++++--
> libsepol/src/policydb_validate.c | 3 +++
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
> index 6682069e..06231574 100644
> --- a/libsepol/include/sepol/policydb/policydb.h
> +++ b/libsepol/include/sepol/policydb/policydb.h
> @@ -218,6 +218,7 @@ typedef struct level_datum {
> mls_level_t *level; /* sensitivity and associated categories */
> unsigned char isalias; /* is this sensitivity an alias for another? */
> unsigned char defined;
> + unsigned char copy; /* whether the level is a non-owned copy (compile time only) */
> } level_datum_t;
>
I don't think this field needs to be added. See below.
> /* Category attributes */
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index f10a8a95..322ab745 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -1390,8 +1390,10 @@ static int sens_destroy(hashtab_key_t key, hashtab_datum_t datum, void *p
> if (key)
> free(key);
> levdatum = (level_datum_t *) datum;
> - mls_level_destroy(levdatum->level);
> - free(levdatum->level);
> + if (!levdatum->copy) {
I believe that the following can be made to work:
if (!levdatum->isalias || levdatum->defined) {
To work, clone_level() and define_level() will need to be modified, so
that defined is not set until right before finishing the call.
Jim
> + mls_level_destroy(levdatum->level);
> + free(levdatum->level);
> + }
> level_datum_destroy(levdatum);
> free(levdatum);
> return 0;
> diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
> index d86f885e..e3af7ccd 100644
> --- a/libsepol/src/policydb_validate.c
> +++ b/libsepol/src/policydb_validate.c
> @@ -623,6 +623,9 @@ static int validate_level_datum(__attribute__ ((unused)) hashtab_key_t k, hashta
> level_datum_t *level = d;
> validate_t *flavors = args;
>
> + if (level->copy)
> + return -1;
> +
> return validate_mls_level(level->level, &flavors[SYM_LEVELS], &flavors[SYM_CATS]);
> }
>
> --
> 2.43.0
>
>
