libsepol: bound type values in type_set_expand negset loop - #532
libsepol: bound type values in type_set_expand negset loop#532netliomax25-code wants to merge 2 commits into
Conversation
type_set_expand() iterates set->negset and dereferences p->type_val_to_struct[i] for each set bit without checking i against p->p_types.nprim. type_val_to_struct is allocated with nprim entries, and the sibling loop over set->types already guards the index with that same nprim check. For a base or module policy, role_read() fills role->types (including negset) from the file via type_set_read(), and policydb_index_others() expands it through policydb_role_cache() and type_set_expand() during policydb_read(), before policydb_validate() runs. A policy whose role negset has a bit at or above nprim then causes an out-of-bounds read of type_val_to_struct. Apply the same bound to the negset loop. The single fix also covers the other callers, expand_rule() and expand_convert_type_set(). Signed-off-by: netliomax25-code <netliomax25@gmail.com>
|
Did this patch hit the mailing list, I don't see it in the archiver? We should probably add a test to prevent regressions. |
Add an expander suite case that drives type_set_expand() with a negset bit one past the declared type count (p_types.nprim). Without the bound in the negset loop this indexes type_val_to_struct out of bounds, which ASAN flags as a heap read overflow; with the bound the call fails cleanly and the test asserts a negative return. Signed-off-by: netliomax25-code <netliomax25@gmail.com>
|
I think @stephensmalley was being nice and posted your patches for you to the list. However, it seems like you have a few patches in the queue, it's probably best to follow the CONTRIBUTING.md file. |
|
I was out of office yesterday and won't get around to these until Monday. note that if you submit them yourself they still need your real name in the Signed-off-by line. |
|
|
Posted to the mailing list with a corrected Signed-off-by line; is there a reason you haven't fixed this on your end? |
Add an expander suite case that drives type_set_expand() with a negset bit one past the declared type count (p_types.nprim). Without the bound in the negset loop this indexes type_val_to_struct out of bounds, which ASAN flags as a heap read overflow; with the bound the call fails cleanly and the test asserts a negative return. Fixes: #532 Signed-off-by: Kartik Kenchi <netliomax25@gmail.com> Acked-by: James Carter <jwcart2@gmail.com>
3 participants
Repro: read a base or module policy whose role negset ebitmap has a bit at or above the declared type count (p_types.nprim).
Cause: the negset loop in type_set_expand indexes type_val_to_struct[i] without the nprim bound the sibling types loop in the same function already applies; this expansion runs from policydb_role_cache during policydb_read, before policydb_validate.
Fix: apply the same nprim bound to the negset loop before the dereference.