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

Bryam Vargas <[email protected]>
Newsgroups org.kernel.feeds.b4-sent,org.kernel.vger.linux-kernel,org.kernel.vger.selinux
Message-ID <[email protected]>
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: [email protected]
Signed-off-by: Bryam Vargas <[email protected]>
---
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.
---
 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 <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.