On Wed, Jun 5, 2024 at 1:38 PM Petr Lautrbach <lautrbach@xxxxxxxxxx> wrote:
>
> Christian Göttsche <cgzones@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
>
>
> This patch seems to be skipped/missed. Others in this series were merged
> or commented.
>
> Petr
>
We ended up going a different direction than this patch. I didn't like
adding another field to the level_datum struct and we eventually ended
up changing the "defined" field to "notdefined" and reworking things
in a better way.
See commit:
fe16f586d5e1da78e4374fdd5ff938524dd792d0
Thanks,
Jim
>
> > ---
> > 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
>
>
