On 09/29/2016 01:50 AM, Nicolas Iooss wrote:
> 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]
>
> Fix this by detecting the overflow before adding new permissions to a
> class.
>
> This bug has been found by fuzzing secilc with american fuzzy lop.
>
I think I'd rather see these bounds checks done earlier in
cil_build_ast.c in cil_gen_class, cil_gen_classcommon. And maybe in
cil_resolve_classcommon as well for the class + classcommon < 32 check.
That way we can avoid alot of unnecessary processing and provide an
error message that includes file/line number information. This check
shouldn't be necessary in cil_binary if we perform the validation earlier.
> Signed-off-by: Nicolas Iooss <nicolas.iooss@xxxxxxx>
> ---
> v2: check that sepol_common->permissions.nprim would not make
> sepol_class->permissions.nprim overflow when added to it.
>
> libsepol/cil/src/cil_binary.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
> index cc73648ad1b7..f83dc2ddd1ee 100644
> --- a/libsepol/cil/src/cil_binary.c
> +++ b/libsepol/cil/src/cil_binary.c
> @@ -332,6 +332,12 @@ int cil_classorder_to_policydb(policydb_t *pdb, const struct cil_db *db, struct
> goto exit;
> }
> }
> + if (sepol_common->permissions.nprim > PERMS_PER_CLASS ||
> + sepol_class->permissions.nprim + sepol_common->permissions.nprim > PERMS_PER_CLASS) {
> + cil_log(CIL_ERR, "Too many permissions in class '%s'\n", cil_class->datum.fqn);
> + rc = SEPOL_ERR;
> + goto exit;
> + }
> sepol_class->comdatum = sepol_common;
> sepol_class->comkey = cil_strdup(key);
> sepol_class->permissions.nprim += sepol_common->permissions.nprim;
> @@ -344,9 +350,15 @@ int cil_classorder_to_policydb(policydb_t *pdb, const struct cil_db *db, struct
>
> for (curr = NODE(cil_class)->cl_head; curr; curr = curr->next) {
> struct cil_perm *cil_perm = curr->data;
> - perm_datum_t *sepol_perm = cil_malloc(sizeof(*sepol_perm));
> - memset(sepol_perm, 0, sizeof(perm_datum_t));
> + perm_datum_t *sepol_perm;
>
> + if (sepol_class->permissions.nprim + 1 > PERMS_PER_CLASS) {
> + cil_log(CIL_ERR, "Too many permissions in class '%s'\n", cil_class->datum.fqn);
> + rc = SEPOL_ERR;
> + goto exit;
> + }
> + sepol_perm = cil_malloc(sizeof(*sepol_perm));
> + memset(sepol_perm, 0, sizeof(perm_datum_t));
> key = cil_strdup(cil_perm->datum.fqn);
> rc = hashtab_insert(sepol_class->permissions.table, key, sepol_perm);
> if (rc != SEPOL_OK) {
>
_______________________________________________
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.
