| Message ID | 20210823204841.175324-1-jwcart2@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | libsepol/cil: Properly check parse tree when printing error messages | expand |
On Mon, Aug 23, 2021 at 4:48 PM James Carter <jwcart2@gmail.com> wrote: > > The function cil_tree_get_next_path() does not check whether the > parse tree node that stores the high-level language file path of a > src_info rule actually exists before trying to read the path. This > can result in a NULL dereference. > > Check that all of the parse tree nodes of a src_info rule exist > before reading the data from them. > > This bug was found by the secilc-fuzzer. > > Signed-off-by: James Carter <jwcart2@gmail.com> > --- > libsepol/cil/src/cil_tree.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c > index 75293005..3fcf5d12 100644 > --- a/libsepol/cil/src/cil_tree.c > +++ b/libsepol/cil/src/cil_tree.c > @@ -62,7 +62,10 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char ** > > while (node) { > if (node->flavor == CIL_NODE && node->data == NULL) { > - if (node->cl_head->data == CIL_KEY_SRC_INFO && node->cl_head->next != NULL && node->cl_head->next->next != NULL) { > + if (node->cl_head->data == CIL_KEY_SRC_INFO) { It should never be NULL, but I should still check that node->cl_head is not NULL here. I have a couple of more fixes for bugs found by the secilc-fuzzer, so I will send an updated patch with those other two patches. Jim > + if (node->cl_head->next == NULL || node->cl_head->next->next == NULL || node->cl_head->next->next->next == NULL) { > + goto exit; > + } > /* Parse Tree */ > *info_kind = node->cl_head->next->data; > rc = cil_string_to_uint32(node->cl_head->next->next->data, hll_line, 10); > -- > 2.31.1 >
diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c index 75293005..3fcf5d12 100644 --- a/libsepol/cil/src/cil_tree.c +++ b/libsepol/cil/src/cil_tree.c @@ -62,7 +62,10 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char ** while (node) { if (node->flavor == CIL_NODE && node->data == NULL) { - if (node->cl_head->data == CIL_KEY_SRC_INFO && node->cl_head->next != NULL && node->cl_head->next->next != NULL) { + if (node->cl_head->data == CIL_KEY_SRC_INFO) { + if (node->cl_head->next == NULL || node->cl_head->next->next == NULL || node->cl_head->next->next->next == NULL) { + goto exit; + } /* Parse Tree */ *info_kind = node->cl_head->next->data; rc = cil_string_to_uint32(node->cl_head->next->next->data, hll_line, 10);
The function cil_tree_get_next_path() does not check whether the parse tree node that stores the high-level language file path of a src_info rule actually exists before trying to read the path. This can result in a NULL dereference. Check that all of the parse tree nodes of a src_info rule exist before reading the data from them. This bug was found by the secilc-fuzzer. Signed-off-by: James Carter <jwcart2@gmail.com> --- libsepol/cil/src/cil_tree.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)