Re: [PATCH 11/15] checkpolicy: fix use-after-free on invalid sens alias

[Petr Lautrbach <lautrbach@redhat.com>]

Christian Göttsche <cgzones@googlemail.com> writes:

> During compilation sensitivity aliases share the level with their prime
> sensitivity, until after the level has been fully defined they are
> deduplicated.  If an error happens by that time the cleanup will free
> the shared level multiple times, leading to a use-after-free.
>
> Make use of the added new member of the struct level_datum.
>
> Example policy:
>
>     class c sid e class c{i}sensitivity S alias L;
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>


This patch seems to be skipped/missed. Others in this series were merged
or commented.

Petr


> ---
>  checkpolicy/fuzz/checkpolicy-fuzzer.c | 7 +++++++
>  checkpolicy/policy_define.c           | 3 +++
>  2 files changed, 10 insertions(+)
>
> diff --git a/checkpolicy/fuzz/checkpolicy-fuzzer.c b/checkpolicy/fuzz/checkpolicy-fuzzer.c
> index 0d749a02..d0221f3f 100644
> --- a/checkpolicy/fuzz/checkpolicy-fuzzer.c
> +++ b/checkpolicy/fuzz/checkpolicy-fuzzer.c
> @@ -134,6 +134,13 @@ static int check_level(hashtab_key_t key, hashtab_datum_t datum, void *arg __att
>  {
>  	const level_datum_t *levdatum = (level_datum_t *) datum;
>  
> +	if (levdatum->copy) {
> +		fprintf(stderr,
> +			"Error:  sensitivity %s is still a copy!\n",
> +			key);
> +		abort();
> +	}
> +
>  	// TODO: drop member defined if proven to be always set
>  	if (!levdatum->isalias && !levdatum->defined) {
>  		fprintf(stderr,
> diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> index 44236797..360cff68 100644
> --- a/checkpolicy/policy_define.c
> +++ b/checkpolicy/policy_define.c
> @@ -756,6 +756,7 @@ int define_sens(void)
>  	}
>  	level_datum_init(datum);
>  	datum->isalias = FALSE;
> +	datum->copy = FALSE;
>  	datum->level = level;
>  
>  	ret = declare_symbol(SYM_LEVELS, id, datum, &value, &value);
> @@ -795,6 +796,7 @@ int define_sens(void)
>  		}
>  		level_datum_init(aliasdatum);
>  		aliasdatum->isalias = TRUE;
> +		aliasdatum->copy = TRUE;
>  		aliasdatum->level = level;
>  
>  		ret = declare_symbol(SYM_LEVELS, id, aliasdatum, NULL, &value);
> @@ -1035,6 +1037,7 @@ static int clone_level(hashtab_key_t key __attribute__ ((unused)), hashtab_datum
>  			return -1;
>  		}
>  		levdatum->level = newlevel;
> +		levdatum->copy = FALSE;
>  	}
>  	return 0;
>  }
> -- 
> 2.43.0