On Wed, Nov 6, 2024 at 2:43 PM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Tue, Nov 5, 2024 at 4:06 PM Christian Göttsche
> <cgoettsche@xxxxxxxxxxxxx> wrote:
> >
> > From: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> >
> > If declare_symbol() returns 1 the id and the datum are already defined
> > and not consumed by the function, so it they must be free'd by the
> > caller.
> >
> > Example policy (generated by fuzzer):
> >
> > class s sid e class s{i}optional{require{bool K;}bool K true;
> >
> > Reported-by: oss-fuzz (issue 377544445)
> > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
>
> Acked-by: James Carter <jwcart2@xxxxxxxxx>
>
Merged.
Thanks,
Jim
> > ---
> > checkpolicy/module_compiler.c | 2 +-
> > checkpolicy/module_compiler.h | 2 +-
> > checkpolicy/policy_define.c | 72 +++++++++++++++++++++++------------
> > 3 files changed, 49 insertions(+), 27 deletions(-)
> >
> > diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
> > index 4efd77bf..3a7ad1bb 100644
> > --- a/checkpolicy/module_compiler.c
> > +++ b/checkpolicy/module_compiler.c
> > @@ -190,7 +190,7 @@ static int create_symbol(uint32_t symbol_type, hashtab_key_t key, hashtab_datum_
> > * not be restricted pointers. */
> > int declare_symbol(uint32_t symbol_type,
> > hashtab_key_t key, hashtab_datum_t datum,
> > - uint32_t * dest_value, uint32_t * datum_value)
> > + uint32_t * dest_value, const uint32_t * datum_value)
> > {
> > avrule_decl_t *decl = stack_top->decl;
> > int ret = create_symbol(symbol_type, key, datum, dest_value, SCOPE_DECL);
> > diff --git a/checkpolicy/module_compiler.h b/checkpolicy/module_compiler.h
> > index e43bc6c0..2fe3455d 100644
> > --- a/checkpolicy/module_compiler.h
> > +++ b/checkpolicy/module_compiler.h
> > @@ -28,7 +28,7 @@ int define_policy(int pass, int module_header_given);
> > */
> > int declare_symbol(uint32_t symbol_type,
> > hashtab_key_t key, hashtab_datum_t datum,
> > - uint32_t * dest_value, uint32_t * datum_value);
> > + uint32_t * dest_value, const uint32_t * datum_value);
> >
> > role_datum_t *declare_role(unsigned char isattr);
> > type_datum_t *declare_type(unsigned char primary, unsigned char isattr);
> > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> > index f8a10154..9aae8378 100644
> > --- a/checkpolicy/policy_define.c
> > +++ b/checkpolicy/policy_define.c
> > @@ -156,9 +156,9 @@ static int id_has_dot(const char *id)
> >
> > int define_class(void)
> > {
> > - char *id = 0;
> > - class_datum_t *datum = 0;
> > - int ret;
> > + char *id = NULL;
> > + class_datum_t *datum = NULL;
> > + int ret = -1;
> > uint32_t value;
> >
> > if (pass == 2) {
> > @@ -175,27 +175,29 @@ int define_class(void)
> > datum = (class_datum_t *) malloc(sizeof(class_datum_t));
> > if (!datum) {
> > yyerror("out of memory");
> > - goto bad;
> > + goto cleanup;
> > }
> > memset(datum, 0, sizeof(class_datum_t));
> > ret = declare_symbol(SYM_CLASSES, id, datum, &value, &value);
> > switch (ret) {
> > case -3:{
> > yyerror("Out of memory!");
> > - goto bad;
> > + goto cleanup;
> > }
> > case -2:{
> > yyerror2("duplicate declaration of class %s", id);
> > - goto bad;
> > + goto cleanup;
> > }
> > case -1:{
> > yyerror("could not declare class here");
> > - goto bad;
> > + goto cleanup;
> > }
> > - case 0:
> > - case 1:{
> > + case 0: {
> > break;
> > }
> > + case 1:{
> > + goto cleanup;
> > + }
> > default:{
> > assert(0); /* should never get here */
> > }
> > @@ -203,12 +205,10 @@ int define_class(void)
> > datum->s.value = value;
> > return 0;
> >
> > - bad:
> > - if (id)
> > - free(id);
> > - if (datum)
> > - free(datum);
> > - return -1;
> > + cleanup:
> > + free(id);
> > + free(datum);
> > + return ret == 1 ? 0 : -1;
> > }
> >
> > int define_permissive(void)
> > @@ -785,8 +785,13 @@ int define_sens(void)
> > yyerror2("could not declare sensitivity level %s here", id);
> > goto bad;
> > }
> > - case 0:
> > + case 0: {
> > + break;
> > + }
> > case 1:{
> > + level_datum_destroy(datum);
> > + free(datum);
> > + free(id);
> > break;
> > }
> > default:{
> > @@ -827,8 +832,13 @@ int define_sens(void)
> > ("could not declare sensitivity alias %s here", id);
> > goto bad_alias;
> > }
> > - case 0:
> > + case 0: {
> > + break;
> > + }
> > case 1:{
> > + level_datum_destroy(aliasdatum);
> > + free(aliasdatum);
> > + free(id);
> > break;
> > }
> > default:{
> > @@ -957,15 +967,20 @@ int define_category(void)
> > yyerror2("could not declare category %s here", id);
> > goto bad;
> > }
> > - case 0:
> > + case 0:{
> > + datum->s.value = value;
> > + break;
> > + }
> > case 1:{
> > + cat_datum_destroy(datum);
> > + free(datum);
> > + free(id);
> > break;
> > }
> > default:{
> > assert(0); /* should never get here */
> > }
> > }
> > - datum->s.value = value;
> >
> > while ((id = queue_remove(id_queue))) {
> > if (id_has_dot(id)) {
> > @@ -981,11 +996,11 @@ int define_category(void)
> > }
> > cat_datum_init(aliasdatum);
> > aliasdatum->isalias = TRUE;
> > - aliasdatum->s.value = datum->s.value;
> > + aliasdatum->s.value = value;
> >
> > ret =
> > declare_symbol(SYM_CATS, id, aliasdatum, NULL,
> > - &datum->s.value);
> > + &value);
> > switch (ret) {
> > case -3:{
> > yyerror("Out of memory!");
> > @@ -1001,8 +1016,13 @@ int define_category(void)
> > ("could not declare category alias %s here", id);
> > goto bad_alias;
> > }
> > - case 0:
> > + case 0:{
> > + break;
> > + }
> > case 1:{
> > + cat_datum_destroy(aliasdatum);
> > + free(aliasdatum);
> > + free(id);
> > break;
> > }
> > default:{
> > @@ -1833,10 +1853,12 @@ int define_bool_tunable(int is_tunable)
> > yyerror2("could not declare boolean %s here", id);
> > goto cleanup;
> > }
> > - case 0:
> > - case 1:{
> > + case 0:{
> > break;
> > }
> > + case 1:{
> > + goto cleanup;
> > + }
> > default:{
> > assert(0); /* should never get here */
> > }
> > @@ -1854,7 +1876,7 @@ int define_bool_tunable(int is_tunable)
> > return 0;
> > cleanup:
> > cond_destroy_bool(id, datum, NULL);
> > - return -1;
> > + return ret == 1 ? 0 : -1;
> > }
> >
> > avrule_t *define_cond_pol_list(avrule_t * avlist, avrule_t * sl)
> > --
> > 2.45.2
> >
> >
