* [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages
@ 2021-08-27 14:19 James Carter
2021-08-27 14:19 ` [PATCH 2/3] libsepol/cil: Reset expandtypeattribute rules when resetting AST James Carter
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: James Carter @ 2021-08-27 14:19 UTC (permalink / raw)
To: selinux; +Cc: nicolas.iooss, 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..e70722ec 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 && node->cl_head->data == CIL_KEY_SRC_INFO) {
+ if (!node->cl_head->next || !node->cl_head->next->next || !node->cl_head->next->next->next) {
+ 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] libsepol/cil: Reset expandtypeattribute rules when resetting AST
2021-08-27 14:19 [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages James Carter
@ 2021-08-27 14:19 ` James Carter
2021-08-27 14:19 ` [PATCH 3/3] libsepol/cil: Properly check for parameter when inserting name James Carter
2021-09-01 19:22 ` [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages Nicolas Iooss
2 siblings, 0 replies; 5+ messages in thread
From: James Carter @ 2021-08-27 14:19 UTC (permalink / raw)
To: selinux; +Cc: nicolas.iooss, James Carter
A list is created to store type attribute datums when resolving an
expandtypeattribute rule and that list needs to be destroyed if the
AST is reset or a memory leak will occur.
Destroy the list storing type attributes datums when resetting
expandtypeattribute rules.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_reset_ast.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/libsepol/cil/src/cil_reset_ast.c b/libsepol/cil/src/cil_reset_ast.c
index 6d1d2da7..0ba075c8 100644
--- a/libsepol/cil/src/cil_reset_ast.c
+++ b/libsepol/cil/src/cil_reset_ast.c
@@ -208,6 +208,11 @@ static void cil_reset_typeattributeset(struct cil_typeattributeset *tas)
cil_list_destroy(&tas->datum_expr, CIL_FALSE);
}
+static void cil_reset_expandtypeattribute(struct cil_expandtypeattribute *expandattr)
+{
+ cil_list_destroy(&expandattr->attr_datums, CIL_FALSE);
+}
+
static void cil_reset_avrule(struct cil_avrule *rule)
{
cil_reset_classperms_list(rule->perms.classperms);
@@ -531,6 +536,9 @@ int __cil_reset_node(struct cil_tree_node *node, __attribute__((unused)) uint32
case CIL_TYPEATTRIBUTESET:
cil_reset_typeattributeset(node->data);
break;
+ case CIL_EXPANDTYPEATTRIBUTE:
+ cil_reset_expandtypeattribute(node->data);
+ break;
case CIL_RANGETRANSITION:
cil_reset_rangetransition(node->data);
break;
@@ -630,7 +638,6 @@ int __cil_reset_node(struct cil_tree_node *node, __attribute__((unused)) uint32
case CIL_CLASSORDER:
case CIL_CATORDER:
case CIL_SENSITIVITYORDER:
- case CIL_EXPANDTYPEATTRIBUTE:
break; /* Nothing to reset */
default:
break;
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] libsepol/cil: Properly check for parameter when inserting name
2021-08-27 14:19 [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages James Carter
2021-08-27 14:19 ` [PATCH 2/3] libsepol/cil: Reset expandtypeattribute rules when resetting AST James Carter
@ 2021-08-27 14:19 ` James Carter
2021-09-01 19:22 ` [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages Nicolas Iooss
2 siblings, 0 replies; 5+ messages in thread
From: James Carter @ 2021-08-27 14:19 UTC (permalink / raw)
To: selinux; +Cc: nicolas.iooss, James Carter
File names for typetransition rules are stored in their own datums.
This allows them to be passed as a parameter, but there needs to be
a check in __cil_insert_name() so that parameter names are not
mistaken for file name strings. This check did not verify that a
matching parameter name had the flavor of CIL_NAME.
Check that the parameter flavor is CIL_NAME and that the paramter
name matches the file name to be stored in the datum.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_resolve_ast.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index 18007324..a4de1c75 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -87,7 +87,8 @@ static struct cil_name * __cil_insert_name(struct cil_db *db, hashtab_key_t key,
if (macro != NULL && macro->params != NULL) {
struct cil_list_item *item;
cil_list_for_each(item, macro->params) {
- if (((struct cil_param*)item->data)->str == key) {
+ struct cil_param *param = item->data;
+ if (param->flavor == CIL_NAME && param->str == key) {
return NULL;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages
2021-08-27 14:19 [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages James Carter
2021-08-27 14:19 ` [PATCH 2/3] libsepol/cil: Reset expandtypeattribute rules when resetting AST James Carter
2021-08-27 14:19 ` [PATCH 3/3] libsepol/cil: Properly check for parameter when inserting name James Carter
@ 2021-09-01 19:22 ` Nicolas Iooss
2021-09-02 16:28 ` James Carter
2 siblings, 1 reply; 5+ messages in thread
From: Nicolas Iooss @ 2021-09-01 19:22 UTC (permalink / raw)
To: James Carter; +Cc: SElinux list
On Fri, Aug 27, 2021 at 4:20 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>
For the 3 patches:
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Thanks,
Nicolas
> ---
> 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..e70722ec 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 && node->cl_head->data == CIL_KEY_SRC_INFO) {
> + if (!node->cl_head->next || !node->cl_head->next->next || !node->cl_head->next->next->next) {
> + 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages
2021-09-01 19:22 ` [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages Nicolas Iooss
@ 2021-09-02 16:28 ` James Carter
0 siblings, 0 replies; 5+ messages in thread
From: James Carter @ 2021-09-02 16:28 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: SElinux list
On Wed, Sep 1, 2021 at 3:22 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Fri, Aug 27, 2021 at 4:20 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>
>
> For the 3 patches:
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
This series has been merged.
Thanks,
Jim
> Thanks,
> Nicolas
>
> > ---
> > 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..e70722ec 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 && node->cl_head->data == CIL_KEY_SRC_INFO) {
> > + if (!node->cl_head->next || !node->cl_head->next->next || !node->cl_head->next->next->next) {
> > + 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
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-02 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 14:19 [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages James Carter
2021-08-27 14:19 ` [PATCH 2/3] libsepol/cil: Reset expandtypeattribute rules when resetting AST James Carter
2021-08-27 14:19 ` [PATCH 3/3] libsepol/cil: Properly check for parameter when inserting name James Carter
2021-09-01 19:22 ` [PATCH 1/3] libsepol/cil: Properly check parse tree when printing error messages Nicolas Iooss
2021-09-02 16:28 ` James Carter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).