Re: [PATCH v2] libsepol: Validate string fields used in various labeling rules

On Thu, Jul 23, 2026 at 12:54 PM James Carter <[email protected]> wrote:
>
> Filename transitions and the ocontexts fs, netif, fs_use,
> ibendport, Xen devicetree, and genfs use strings in a name or type
> field used to identify what to label.
>
> Check that these strings do not contain any control characters to
> protect against maliciously crafted policies.
>
> Signed-off-by: James Carter <[email protected]>

Acked-by: Stephen Smalley <[email protected]>

> ---
> v2: Added another check in validate_genfs() and a check in
>      validate_filename_trans_rules
>
>  libsepol/src/policydb_validate.c | 35 +++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
> index cde0b8d6..e7d79871 100644
> --- a/libsepol/src/policydb_validate.c
> +++ b/libsepol/src/policydb_validate.c
> @@ -1,4 +1,5 @@
>
> +#include <ctype.h>
>  #include <sepol/policydb/conditional.h>
>  #include <sepol/policydb/ebitmap.h>
>  #include <sepol/policydb/polcaps.h>
> @@ -129,6 +130,23 @@ bad:
>         return -1;
>  }
>
> +static int validate_string_field(const char *s)
> +{
> +       if (!s || !*s)
> +               goto bad;
> +
> +       while (*s) {
> +               if (iscntrl((unsigned char)*s))
> +                       goto bad;
> +               s++;
> +       }
> +
> +       return 0;
> +
> +bad:
> +       return -1;
> +}
> +
>  static int validate_ebitmap(const ebitmap_t *map, const validate_t *flavor)
>  {
>         if (ebitmap_length(map) > 0 &&
> @@ -1436,6 +1454,8 @@ static int validate_filename_trans(hashtab_key_t k, hashtab_datum_t d,
>                 goto bad;
>         if (validate_value(ftk->tclass, &flavors[SYM_CLASSES]))
>                 goto bad;
> +       if (validate_string_field(ftk->name))
> +               goto bad;
>         if (!ftd)
>                 goto bad;
>         for (; ftd; ftd = ftd->next) {
> @@ -1505,7 +1525,7 @@ static int validate_ocontexts(sepol_handle_t *handle, const policydb_t *p,
>                                         if (validate_context(&octx->context[1],
>                                                              flavors, p->mls))
>                                                 goto bad;
> -                                       if (!octx->u.name)
> +                                       if (validate_string_field(octx->u.name))
>                                                 goto bad;
>                                         break;
>                                 case OCON_PORT:
> @@ -1522,7 +1542,7 @@ static int validate_ocontexts(sepol_handle_t *handle, const policydb_t *p,
>                                         default:
>                                                 goto bad;
>                                         }
> -                                       if (!octx->u.name)
> +                                       if (validate_string_field(octx->u.name))
>                                                 goto bad;
>                                         break;
>                                 case OCON_IBPKEY:
> @@ -1533,7 +1553,8 @@ static int validate_ocontexts(sepol_handle_t *handle, const policydb_t *p,
>                                 case OCON_IBENDPORT:
>                                         if (octx->u.ibendport.port == 0)
>                                                 goto bad;
> -                                       if (!octx->u.ibendport.dev_name)
> +                                       if (validate_string_field(
> +                                                   octx->u.ibendport.dev_name))
>                                                 goto bad;
>                                         break;
>                                 }
> @@ -1560,7 +1581,7 @@ static int validate_ocontexts(sepol_handle_t *handle, const policydb_t *p,
>                                                 goto bad;
>                                         break;
>                                 case OCON_XEN_DEVICETREE:
> -                                       if (!octx->u.name)
> +                                       if (validate_string_field(octx->u.name))
>                                                 goto bad;
>                                         break;
>                                 }
> @@ -1590,9 +1611,11 @@ static int validate_genfs(sepol_handle_t *handle, const policydb_t *p,
>                             validate_value(octx->v.sclass,
>                                            &flavors[SYM_CLASSES]))
>                                 goto bad;
> +                       if (validate_string_field(octx->u.name))
> +                               goto bad;
>                 }
>
> -               if (!genfs->fstype)
> +               if (validate_string_field(genfs->fstype))
>                         goto bad;
>         }
>
> @@ -1728,6 +1751,8 @@ validate_filename_trans_rules(sepol_handle_t *handle,
>                         goto bad;
>                 if (validate_simpletype(filename_trans->otype, p, flavors))
>                         goto bad;
> +               if (validate_string_field(filename_trans->name))
> +                       goto bad;
>
>                 /* currently only the RULE_SELF flag can be set */
>                 switch (filename_trans->flags) {
> --
> 2.55.0
>