| Message ID | 20211011162533.53404-14-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | libsepol: add fuzzer for reading binary policies | expand |
On Mon, Oct 11, 2021 at 12:41 PM Christian Göttsche <cgzones@googlemail.com> wrote: > > Validate the level map of the policy to ensure no level refers to a non > existent category. > > READ of size 8 at 0x602000000c58 thread T0 > #0 0x568d2c in cats_ebitmap_len ./libsepol/src/kernel_to_conf.c:1003:14 > #1 0x568d2c in cats_ebitmap_to_str ./libsepol/src/kernel_to_conf.c:1038:19 > #2 0x55e371 in write_level_rules_to_conf ./libsepol/src/kernel_to_conf.c:1106:11 > #3 0x55e371 in write_mls_rules_to_conf ./libsepol/src/kernel_to_conf.c:1140:7 > #4 0x55adb1 in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3103:7 > #5 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9 > #6 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o > #7 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o > #8 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o > #9 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) > #10 0x7f741d0d67ec in __libc_start_main csu/../csu/libc-start.c:332:16 > #11 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > libsepol/src/policydb_validate.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c > index 5804d247..ca0dcca3 100644 > --- a/libsepol/src/policydb_validate.c > +++ b/libsepol/src/policydb_validate.c > @@ -310,6 +310,29 @@ bad: > return -1; > } > > +static int validate_mls_level(mls_level_t *level, validate_t *sens, validate_t *cats) > +{ > + if (level->sens == 0) > + return 0; > + if (validate_value(level->sens, sens)) > + goto bad; > + if (validate_ebitmap(&level->cat, cats)) > + goto bad; > + > + return 0; > + > + bad: > + return -1; > +} > + > +static int validate_level(__attribute__ ((unused))hashtab_key_t k, hashtab_datum_t d, void *args) I would prefer validate_level_datum for the name of this function. > +{ > + level_datum_t *level = d; > + validate_t *flavors = args; > + > + return validate_mls_level(level->level, &flavors[SYM_LEVELS], &flavors[SYM_CATS]); > +} > + > static int validate_datum_arrays(sepol_handle_t *handle, policydb_t *p, validate_t flavors[]) > { > unsigned int i; > @@ -368,6 +391,9 @@ static int validate_datum_arrays(sepol_handle_t *handle, policydb_t *p, validate > } > } > > + if (hashtab_map(p->p_levels.table, validate_level, flavors)) > + goto bad; > + This should not be in validate_datum_arrays(). This also applies to patches 17 and 33. I think the best course would be to pull out the validate_X_datum() calls out of validate_datum_arrays() so that the only thing being checked is that the gaps in the X_val_to_struct arrays correspond to any gaps in the X_val_to_name arrays. Have a function called validate_global_symtabs() or something that walks all of the symtabs and does the checks on the datums (including aliases). So this hashtab_map() call would be in the function, along with calls to hashtab_map using modified validate_X_datum() functions that can be used with hashtab_map(). I think something like: if (hashtab_map(p->p_commons.table, validate_common_datum, flavors)) goto bad; if (hashtab_map(p->p_classes.table, validate_class_datum, flavors)) goto bad; if (hashtab_map(p->p_roles.table, validate_role_datum, flavors)) goto bad; if (hashtab_map(p->p_types.table, validate_type_datum, flavors)) goto bad; if (hashtab_map(p->p_users.table, validate_user_datum, flavors)) goto bad; if (hashtab_map(p->p_bools.table, validate_bool_datum, flavors)) goto bad; if (hashtab_map(p->p_levels.table, validate_level_datum, flavors)) goto bad; if (hashtab_map(p->p_cats.table, validate_cat_datum, flavors)) goto bad; Thanks, Jim
diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c index 5804d247..ca0dcca3 100644 --- a/libsepol/src/policydb_validate.c +++ b/libsepol/src/policydb_validate.c @@ -310,6 +310,29 @@ bad: return -1; } +static int validate_mls_level(mls_level_t *level, validate_t *sens, validate_t *cats) +{ + if (level->sens == 0) + return 0; + if (validate_value(level->sens, sens)) + goto bad; + if (validate_ebitmap(&level->cat, cats)) + goto bad; + + return 0; + + bad: + return -1; +} + +static int validate_level(__attribute__ ((unused))hashtab_key_t k, hashtab_datum_t d, void *args) +{ + level_datum_t *level = d; + validate_t *flavors = args; + + return validate_mls_level(level->level, &flavors[SYM_LEVELS], &flavors[SYM_CATS]); +} + static int validate_datum_arrays(sepol_handle_t *handle, policydb_t *p, validate_t flavors[]) { unsigned int i; @@ -368,6 +391,9 @@ static int validate_datum_arrays(sepol_handle_t *handle, policydb_t *p, validate } } + if (hashtab_map(p->p_levels.table, validate_level, flavors)) + goto bad; + return 0; bad:
Validate the level map of the policy to ensure no level refers to a non existent category. READ of size 8 at 0x602000000c58 thread T0 #0 0x568d2c in cats_ebitmap_len ./libsepol/src/kernel_to_conf.c:1003:14 #1 0x568d2c in cats_ebitmap_to_str ./libsepol/src/kernel_to_conf.c:1038:19 #2 0x55e371 in write_level_rules_to_conf ./libsepol/src/kernel_to_conf.c:1106:11 #3 0x55e371 in write_mls_rules_to_conf ./libsepol/src/kernel_to_conf.c:1140:7 #4 0x55adb1 in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3103:7 #5 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9 #6 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o #7 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o #8 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o #9 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) #10 0x7f741d0d67ec in __libc_start_main csu/../csu/libc-start.c:332:16 #11 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- libsepol/src/policydb_validate.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)