Re: [PATCH] selinux: reject a permission value exceeding the class permission count

[Stephen Smalley]

On Fri, Jul 24, 2026 at 10:47 AM Bryam Vargas via B4 Relay
<devnull+hexlabsecurity.proton.me@xxxxxxxxxx> wrote:
>
> From: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
>
> perm_read() bounds a permission value by SEL_VEC_MAX but never by the
> owning class or common's nprim, which is taken verbatim from the policy
> image.  security_get_permissions() then writes perms[value - 1] into an
> nprim-sized kcalloc() array, so a class declaring fewer permissions than
> its largest permission value drives an out-of-bounds heap write.  The
> top-level symbol tables are validated this way; the nested per-class
> permission table is not.
>
> Reject a permission whose value exceeds nprim, which is already set when
> perm_read() runs.  Well-formed policies are unaffected.
>
> Fixes: 55fcf09b3fe4 ("selinux: add support for querying object classes and permissions from the running policy")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
> ---
> Reproducer and A/B verification (below the --- so git am drops it):
>
> A binary policy whose "process" class carries permissions.nprim = 1 while
> keeping its 31 permissions (values 1..31) is accepted by the parser.  On
> load, security_get_permissions() allocates a one-entry array and writes
> perms[value - 1] for each permission -- a heap slab-out-of-bounds write.
>
> Tested on a KASAN kernel, loading via /sys/fs/selinux/load:
>
>   Build A (without this patch): loading the malformed policy ->
>     BUG: KASAN: slab-out-of-bounds in get_permissions_callback
>     Write of size 8 ... security_get_permissions()
>     (followed by a GPF once an adjacent object's pointer is overwritten)
>
>   Build B (with this patch): the same policy is rejected at parse time
>     (-EINVAL); no KASAN report.
>
>   Control (without this patch): a well-formed policy (nprim = 31) loads
>     cleanly, no KASAN report -- the fault is specific to nprim < value.
>
> Reachable by a process with CAP_MAC_ADMIN writing a crafted policy to
> /sys/fs/selinux/load; SELinux policy load is not namespaced.

Thanks for the report and patch - one point of clarification to your
statement above:
SELinux doesn't check CAP_MAC_ADMIN for policy load since SELinux predates
CAP_MAC_ADMIN and defines and checks its own permission checks (in this case,
security:load_policy) for its operations. You need to be able to open
/sys/fs/selinux/load
with write access so that's typically uid-0 or CAP_DAC_OVERRIDE along with the
SELinux load_policy permission check.

> ---
>  security/selinux/ss/policydb.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index ead504a639e3..6973b68d9782 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -1175,6 +1175,9 @@ static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *f
>         rc = -EINVAL;
>         if (perdatum->value < 1 || perdatum->value > SEL_VEC_MAX)
>                 goto bad;
> +       /* the value indexes an nprim-sized array in security_get_permissions() */
> +       if (perdatum->value > s->nprim)
> +               goto bad;
>
>         rc = str_read(&key, GFP_KERNEL, fp, len);
>         if (rc)
>
> ---
> base-commit: 48a5a7ab8d6ab7090564339e039c421f315de912
> change-id: 20260724-b4-disp-ec8ac9f6-576c3f177a04
>
> Best regards,
> --
> Bryam Vargas <hexlabsecurity@xxxxxxxxx>
>
>