Re: [PATCH] libsepol/cil: Check for too many permissions in classes and commons

[James Carter <jwcart2@xxxxxxxxxxxxx>]

On 09/29/2016 02:38 PM, Steve Lawrence wrote:
> On 09/29/2016 02:07 PM, James Carter wrote:
> > Fixes bug found by Nicolas Iooss as described below in the way suggested by Steve Lawrence.
> >
> > Nicolass reported:
> >
> > When compiling a CIL policy with more than 32 items in a class (e.g. in
> > (class capability (chown ...)) with many items),
> > cil_classorder_to_policydb() overflows perm_value_to_cil[class_index]
> > array. As this array is allocated on the heap through
> > calloc(PERMS_PER_CLASS+1, sizeof(...)), this makes secilc crash with the
> > following message:
> >
> >     *** Error in `/usr/bin/secilc': double free or corruption (!prev): 0x000000000062be80 ***
> >     ======= Backtrace: =========
> >     /usr/lib/libc.so.6(+0x70c4b)[0x7ffff76a7c4b]
> >     /usr/lib/libc.so.6(+0x76fe6)[0x7ffff76adfe6]
> >     /usr/lib/libc.so.6(+0x777de)[0x7ffff76ae7de]
> >     /lib/libsepol.so.1(+0x14fbda)[0x7ffff7b24bda]
> >     /lib/libsepol.so.1(+0x152db8)[0x7ffff7b27db8]
> >     /lib/libsepol.so.1(cil_build_policydb+0x63)[0x7ffff7af8723]
> >     /usr/bin/secilc[0x40273b]
> >     /usr/lib/libc.so.6(__libc_start_main+0xf1)[0x7ffff7657291]
> >     /usr/bin/secilc[0x402f7a]
> >
> > This bug has been found by fuzzing secilc with american fuzzy lop.
> >
> > Signed-off-by: James Carter <jwcart2@xxxxxxxxxxxxx>
> > ---
>
> Looks good to me.

It has been applied.

Jim

> >  libsepol/cil/src/cil_build_ast.c   | 9 +++++++++
> >  libsepol/cil/src/cil_internal.h    | 2 ++
> >  libsepol/cil/src/cil_resolve_ast.c | 6 ++++++
> >  3 files changed, 17 insertions(+)
> >
> > diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> > index 1505873..a96c2a9 100644
> > --- a/libsepol/cil/src/cil_build_ast.c
> > +++ b/libsepol/cil/src/cil_build_ast.c
> > @@ -377,6 +377,11 @@ int cil_gen_class(struct cil_db *db, struct cil_tree_node *parse_current, struct
> >  		if (rc != SEPOL_OK) {
> >  			goto exit;
> >  		}
> > +		if (class->num_perms > CIL_PERMS_PER_CLASS) {
> > +			cil_tree_log(parse_current, CIL_ERR, "Too many permissions in class '%s'", class->datum.name);
> > +			goto exit;
> > +		}
> > +
> >  	}
> >
> >  	return SEPOL_OK;
> > @@ -939,6 +944,10 @@ int cil_gen_common(struct cil_db *db, struct cil_tree_node *parse_current, struc
> >  	if (rc != SEPOL_OK) {
> >  		goto exit;
> >  	}
> > +	if (common->num_perms > CIL_PERMS_PER_CLASS) {
> > +		cil_tree_log(parse_current, CIL_ERR, "Too many permissions in common '%s'", common->datum.name);
> > +		goto exit;
> > +	}
> >
> >  	return SEPOL_OK;
> >
> > diff --git a/libsepol/cil/src/cil_internal.h b/libsepol/cil/src/cil_internal.h
> > index 5875dc9..03672bb 100644
> > --- a/libsepol/cil/src/cil_internal.h
> > +++ b/libsepol/cil/src/cil_internal.h
> > @@ -37,6 +37,7 @@
> >
> >  #include <sepol/policydb/services.h>
> >  #include <sepol/policydb/policydb.h>
> > +#include <sepol/policydb/flask_types.h>
> >
> >  #include <cil/cil.h>
> >
> > @@ -270,6 +271,7 @@ enum cil_sym_array {
> >  extern int cil_sym_sizes[CIL_SYM_ARRAY_NUM][CIL_SYM_NUM];
> >
> >  #define CIL_CLASS_SYM_SIZE	256
> > +#define CIL_PERMS_PER_CLASS (sizeof(sepol_access_vector_t) * 8)
> >
> >  struct cil_db {
> >  	struct cil_tree *parse;
> > diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
> > index 8348d57..917adf8 100644
> > --- a/libsepol/cil/src/cil_resolve_ast.c
> > +++ b/libsepol/cil/src/cil_resolve_ast.c
> > @@ -717,6 +717,10 @@ int cil_resolve_classcommon(struct cil_tree_node *current, void *extra_args)
> >  	cil_symtab_map(&class->perms, __class_update_perm_values, &common->num_perms);
> >
> >  	class->num_perms += common->num_perms;
> > +	if (class->num_perms > CIL_PERMS_PER_CLASS) {
> > +		cil_tree_log(current, CIL_ERR, "Too many permissions in class '%s' when including common permissions", class->datum.name);
> > +		goto exit;
> > +	}
> >
> >  	return SEPOL_OK;
> >
> > @@ -1447,6 +1451,7 @@ int cil_resolve_classorder(struct cil_tree_node *current, void *extra_args)
> >  	return SEPOL_OK;
> >
> >  exit:
> > +	cil_list_destroy(&new, CIL_FALSE);
> >  	return rc;
> >  }
> >
> > @@ -3919,6 +3924,7 @@ exit:
> >  	__cil_ordered_lists_destroy(&extra_args.catorder_lists);
> >  	__cil_ordered_lists_destroy(&extra_args.sensitivityorder_lists);
> >  	cil_list_destroy(&extra_args.in_list, CIL_FALSE);
> > +	cil_list_destroy(&extra_args.unordered_classorder_lists, CIL_FALSE);
> >
> >  	return rc;
> >  }


--
James Carter <jwcart2@xxxxxxxxxxxxx>
National Security Agency
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.