On Tue, Mar 16, 2021 at 4:49 PM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:
>
> On Tue, Mar 16, 2021 at 5:51 PM James Carter <jwcart2@xxxxxxxxx> wrote:
> >
> > In CIL, blocks, optionals, and macros share the same symbol table so
> > that the targets of "in" statements can be located. Because of this,
> > they cannot have the same name in the same namespace, but, because
> > they do not show up in the final policy, they can have the same name
> > as long as they are in different namespaces. Unfortunately, when
> > copying from one namespace to another, no check was being done to see
> > if there was a conflict.
> >
> > When copying blocks, optionals, and macros, if a datum is found in
> > the destination namespace, then there is a conflict with a previously
> > declared block, optional, or macro, so exit with an error.
> >
> > Reported-by: Nicolas Iooss <nicolas.iooss@xxxxxxx>
> > Reported-by: Evgeny Vereshchagin <evvers@xxxxx>
> > Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
>
> I confirm this patch fixes the reported bug, and makes secilc no
> longer crashes on the non-reduced test case found by OSS-Fuzz.
>
> Acked-by: Nicolas Iooss <nicolas.iooss@xxxxxxx>
>
Merged.
Thanks,
Jim
> Thanks!
> Nicolas
>
> > ---
> > libsepol/cil/src/cil_copy_ast.c | 89 +++++++++------------------------
> > 1 file changed, 25 insertions(+), 64 deletions(-)
> >
> > diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> > index c9aada9d..ed967861 100644
> > --- a/libsepol/cil/src/cil_copy_ast.c
> > +++ b/libsepol/cil/src/cil_copy_ast.c
> > @@ -100,16 +100,17 @@ int cil_copy_block(__attribute__((unused)) struct cil_db *db, void *data, void *
> > struct cil_block *orig = data;
> > char *key = orig->datum.name;
> > struct cil_symtab_datum *datum = NULL;
> > + struct cil_block *new;
> >
> > cil_symtab_get_datum(symtab, key, &datum);
> > - if (datum == NULL) {
> > - struct cil_block *new;
> > - cil_block_init(&new);
> > - *copy = new;
> > - } else {
> > - *copy = datum;;
> > + if (datum != NULL) {
> > + cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> > + return SEPOL_ERR;
> > }
> >
> > + cil_block_init(&new);
> > + *copy = new;
> > +
> > return SEPOL_OK;
> > }
> >
> > @@ -1509,64 +1510,22 @@ int cil_copy_macro(__attribute__((unused)) struct cil_db *db, void *data, void *
> > struct cil_macro *orig = data;
> > char *key = orig->datum.name;
> > struct cil_symtab_datum *datum = NULL;
> > + struct cil_macro *new;
> >
> > cil_symtab_get_datum(symtab, key, &datum);
> > - if (datum == NULL) {
> > - struct cil_macro *new;
> > - cil_macro_init(&new);
> > - if (orig->params != NULL) {
> > - cil_copy_list(orig->params, &new->params);
> > - }
> > -
> > - *copy = new;
> > -
> > - } else {
> > - struct cil_list_item *curr_orig = NULL;
> > - struct cil_list_item *curr_new = NULL;
> > - struct cil_param *param_orig = NULL;
> > - struct cil_param *param_new = NULL;
> > -
> > - if (((struct cil_macro*)datum)->params != NULL) {
> > - curr_new = ((struct cil_macro*)datum)->params->head;
> > - }
> > -
> > - if (orig->params != NULL) {
> > - curr_orig = orig->params->head;
> > - }
> > -
> > - if (curr_orig != NULL && curr_new != NULL) {
> > - while (curr_orig != NULL) {
> > - if (curr_new == NULL) {
> > - goto exit;
> > - }
> > -
> > - param_orig = (struct cil_param*)curr_orig->data;
> > - param_new = (struct cil_param*)curr_new->data;
> > - if (param_orig->str != param_new->str) {
> > - goto exit;
> > - } else if (param_orig->flavor != param_new->flavor) {
> > - goto exit;
> > - }
> > -
> > - curr_orig = curr_orig->next;
> > - curr_new = curr_new->next;
> > - }
> > -
> > - if (curr_new != NULL) {
> > - goto exit;
> > - }
> > - } else if (!(curr_orig == NULL && curr_new == NULL)) {
> > - goto exit;
> > - }
> > + if (datum != NULL) {
> > + cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> > + return SEPOL_ERR;
> > + }
> >
> > - *copy = datum;
> > + cil_macro_init(&new);
> > + if (orig->params != NULL) {
> > + cil_copy_list(orig->params, &new->params);
> > }
> >
> > - return SEPOL_OK;
> > + *copy = new;
> >
> > -exit:
> > - cil_log(CIL_INFO, "cil_copy_macro: macro cannot be redefined\n");
> > - return SEPOL_ERR;
> > + return SEPOL_OK;
> > }
> >
> > int cil_copy_optional(__attribute__((unused)) struct cil_db *db, void *data, void **copy, symtab_t *symtab)
> > @@ -1574,16 +1533,17 @@ int cil_copy_optional(__attribute__((unused)) struct cil_db *db, void *data, voi
> > struct cil_optional *orig = data;
> > char *key = orig->datum.name;
> > struct cil_symtab_datum *datum = NULL;
> > + struct cil_optional *new;
> >
> > cil_symtab_get_datum(symtab, key, &datum);
> > - if (datum == NULL) {
> > - struct cil_optional *new;
> > - cil_optional_init(&new);
> > - *copy = new;
> > - } else {
> > - *copy = datum;
> > + if (datum != NULL) {
> > + cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
> > + return SEPOL_ERR;
> > }
> >
> > + cil_optional_init(&new);
> > + *copy = new;
> > +
> > return SEPOL_OK;
> > }
> >
> > @@ -2122,6 +2082,7 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
> > args->dest = new;
> > }
> > } else {
> > + cil_tree_log(orig, CIL_ERR, "Problem copying %s node", cil_node_to_string(orig));
> > goto exit;
> > }
> >
> > --
> > 2.26.2
> >
>
