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>
> ---
> 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
>
>
