[PATCH] libsepol/cil: Properly check parse tree when printing error messages

[PATCH] libsepol/cil: Properly check parse tree when printing error messages

[James Carter]

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) {
+				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

Re: [PATCH] libsepol/cil: Properly check parse tree when printing error messages

[James Carter]

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
>