diff mbox series

[v2] libsepol: Fix buffer overflow when using sepol_av_to_string()

Message ID 20240311211946.2691879-1-jwcart2@gmail.com (mailing list archive)
State Accepted
Commit c205b924e280
Delegated to: Petr Lautrbach
Headers show
Series [v2] libsepol: Fix buffer overflow when using sepol_av_to_string() | expand

Commit Message

James Carter March 11, 2024, 9:19 p.m. UTC
The function sepol_av_to_string() normally returns a list of
permissions with a space at the beginning, but it will return '\0'
if there are no permissions. Unfortunately, functions in
kernel_to_cil, kernel_to_conf, and module_to_cil assume there is a
space at the beginning and skip the space by using "perms+1".

In kernel_to_cil, kernel_to_conf, and module_to_cil, check for the
permission string being '\0' and return an error if it is.

Reported-by: oss-fuzz (issue 67276)
Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/src/kernel_to_cil.c  | 11 +++++++++++
 libsepol/src/kernel_to_conf.c | 11 +++++++++++
 libsepol/src/module_to_cil.c  | 12 ++++++++++++
 3 files changed, 34 insertions(+)

Comments

James Carter March 20, 2024, 8:05 p.m. UTC | #1
On Mon, Mar 11, 2024 at 5:19 PM James Carter <jwcart2@gmail.com> wrote:
>
> The function sepol_av_to_string() normally returns a list of
> permissions with a space at the beginning, but it will return '\0'
> if there are no permissions. Unfortunately, functions in
> kernel_to_cil, kernel_to_conf, and module_to_cil assume there is a
> space at the beginning and skip the space by using "perms+1".
>
> In kernel_to_cil, kernel_to_conf, and module_to_cil, check for the
> permission string being '\0' and return an error if it is.
>
> Reported-by: oss-fuzz (issue 67276)
> Signed-off-by: James Carter <jwcart2@gmail.com>

This patch has been merged.
Jim

> ---
>  libsepol/src/kernel_to_cil.c  | 11 +++++++++++
>  libsepol/src/kernel_to_conf.c | 11 +++++++++++
>  libsepol/src/module_to_cil.c  | 12 ++++++++++++
>  3 files changed, 34 insertions(+)
>
> diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c
> index a081915e..e20ba4af 100644
> --- a/libsepol/src/kernel_to_cil.c
> +++ b/libsepol/src/kernel_to_cil.c
> @@ -302,6 +302,12 @@ static int class_constraint_rules_to_strs(struct policydb *pdb, char *classkey,
>                         rc = -1;
>                         goto exit;
>                 }
> +               if (*perms == '\0') {
> +                       ERR(NULL, "No permisisons in permission string");
> +                       free(perms);
> +                       rc = -1;
> +                       goto exit;
> +               }
>
>                 if (is_mls) {
>                         key_word = "mlsconstrain";
> @@ -1775,6 +1781,11 @@ static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat
>                         ERR(NULL, "Failed to generate permission string");
>                         goto exit;
>                 }
> +               if (*perms == '\0') {
> +                       ERR(NULL, "No permisisons in permission string");
> +                       free(perms);
> +                       goto exit;
> +               }
>                 rule = create_str("(%s %s %s (%s (%s)))",
>                                   flavor, src, tgt, class, perms+1);
>                 free(perms);
> diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
> index 956fb6e8..5860a513 100644
> --- a/libsepol/src/kernel_to_conf.c
> +++ b/libsepol/src/kernel_to_conf.c
> @@ -297,6 +297,12 @@ static int class_constraint_rules_to_strs(struct policydb *pdb, char *classkey,
>                         rc = -1;
>                         goto exit;
>                 }
> +               if (*perms == '\0') {
> +                       ERR(NULL, "No permisisons in permission string");
> +                       free(perms);
> +                       rc = -1;
> +                       goto exit;
> +               }
>                 if (strchr(perms, ' ')) {
>                         perm_prefix = "{ ";
>                         perm_suffix = " }";
> @@ -1741,6 +1747,11 @@ static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat
>                         ERR(NULL, "Failed to generate permission string");
>                         goto exit;
>                 }
> +               if (*permstring == '\0') {
> +                       ERR(NULL, "No permisisons in permission string");
> +                       free(permstring);
> +                       goto exit;
> +               }
>                 rule = create_str("%s %s %s:%s { %s };",
>                                   flavor, src, tgt, class, permstring+1);
>                 free(permstring);
> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
> index 6699a46b..3b3480bf 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -593,6 +593,12 @@ static int avrule_to_cil(int indent, struct policydb *pdb, uint32_t type, const
>                                 rc = -1;
>                                 goto exit;
>                         }
> +                       if (*perms == '\0') {
> +                               ERR(NULL, "No permissions in permission string");
> +                               free(perms);
> +                               rc = -1;
> +                               goto exit;
> +                       }
>                         cil_println(indent, "(%s %s %s (%s (%s)))",
>                                         rule, src, tgt,
>                                         pdb->p_class_val_to_name[classperm->tclass - 1],
> @@ -1973,6 +1979,12 @@ static int constraints_to_cil(int indent, struct policydb *pdb, char *classkey,
>                                 rc = -1;
>                                 goto exit;
>                         }
> +                       if (*perms == '\0') {
> +                               ERR(NULL, "No permissions in permission string");
> +                               free(perms);
> +                               rc = -1;
> +                               goto exit;
> +                       }
>                         cil_println(indent, "(%sconstrain (%s (%s)) %s)", mls, classkey, perms + 1, expr);
>                         free(perms);
>                 } else {
> --
> 2.44.0
>
diff mbox series

Patch

diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c
index a081915e..e20ba4af 100644
--- a/libsepol/src/kernel_to_cil.c
+++ b/libsepol/src/kernel_to_cil.c
@@ -302,6 +302,12 @@  static int class_constraint_rules_to_strs(struct policydb *pdb, char *classkey,
 			rc = -1;
 			goto exit;
 		}
+		if (*perms == '\0') {
+			ERR(NULL, "No permisisons in permission string");
+			free(perms);
+			rc = -1;
+			goto exit;
+		}
 
 		if (is_mls) {
 			key_word = "mlsconstrain";
@@ -1775,6 +1781,11 @@  static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat
 			ERR(NULL, "Failed to generate permission string");
 			goto exit;
 		}
+		if (*perms == '\0') {
+			ERR(NULL, "No permisisons in permission string");
+			free(perms);
+			goto exit;
+		}
 		rule = create_str("(%s %s %s (%s (%s)))",
 				  flavor, src, tgt, class, perms+1);
 		free(perms);
diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
index 956fb6e8..5860a513 100644
--- a/libsepol/src/kernel_to_conf.c
+++ b/libsepol/src/kernel_to_conf.c
@@ -297,6 +297,12 @@  static int class_constraint_rules_to_strs(struct policydb *pdb, char *classkey,
 			rc = -1;
 			goto exit;
 		}
+		if (*perms == '\0') {
+			ERR(NULL, "No permisisons in permission string");
+			free(perms);
+			rc = -1;
+			goto exit;
+		}
 		if (strchr(perms, ' ')) {
 			perm_prefix = "{ ";
 			perm_suffix = " }";
@@ -1741,6 +1747,11 @@  static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat
 			ERR(NULL, "Failed to generate permission string");
 			goto exit;
 		}
+		if (*permstring == '\0') {
+			ERR(NULL, "No permisisons in permission string");
+			free(permstring);
+			goto exit;
+		}
 		rule = create_str("%s %s %s:%s { %s };",
 				  flavor, src, tgt, class, permstring+1);
 		free(permstring);
diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 6699a46b..3b3480bf 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -593,6 +593,12 @@  static int avrule_to_cil(int indent, struct policydb *pdb, uint32_t type, const
 				rc = -1;
 				goto exit;
 			}
+			if (*perms == '\0') {
+				ERR(NULL, "No permissions in permission string");
+				free(perms);
+				rc = -1;
+				goto exit;
+			}
 			cil_println(indent, "(%s %s %s (%s (%s)))",
 					rule, src, tgt,
 					pdb->p_class_val_to_name[classperm->tclass - 1],
@@ -1973,6 +1979,12 @@  static int constraints_to_cil(int indent, struct policydb *pdb, char *classkey,
 				rc = -1;
 				goto exit;
 			}
+			if (*perms == '\0') {
+				ERR(NULL, "No permissions in permission string");
+				free(perms);
+				rc = -1;
+				goto exit;
+			}
 			cil_println(indent, "(%sconstrain (%s (%s)) %s)", mls, classkey, perms + 1, expr);
 			free(perms);
 		} else {