| Message ID | 20240122135507.63506-11-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Petr Lautrbach |
| Headers | show |
| Series | [01/15] checkpolicy: add libfuzz based fuzzer | expand |
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
On Wed, Jun 5, 2024 at 1:38 PM Petr Lautrbach <lautrbach@redhat.com> wrote: > > 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 > 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 > >
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; }
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> --- checkpolicy/fuzz/checkpolicy-fuzzer.c | 7 +++++++ checkpolicy/policy_define.c | 3 +++ 2 files changed, 10 insertions(+)