> On Jul 1, 2022, at 5:26 AM, Christian Göttsche <cgzones@xxxxxxxxxxxxxx> wrote: > > On Fri, 1 Jul 2022 at 03:58, Karl MacMillan <karl@xxxxxxxxxxxxxxxxxxxxxx> wrote: >> >> Hi Nicolas, >> >> I believe these are described on page 19 of the old "A Security Policy >> Configuration for the Security-Enhanced Linux" [1]. > > Quote from 7.2 File System Contexts: > > Currently, this configuration is unused. > >> There is still support for these in the kernel [2], so it seems unwise to me to drop >> them even if they are not used in policies. > > git log -S OCON_FS lists > > 335c818c5a7a can: mcp251xfd: move chip FIFO init into separate file > 55e5b97f003e can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN > 875347fe5756 can: mcp25xxfd: add regmap infrastructure > cee74f47a6ba SELinux: allow userspace to read policy back out of the kernel > 1da177e4c3f4 (tag: v2.6.12-rc2) Linux-2.6.12-rc2 > > and grepping the source shows > > $ grep -Rw OCON_FS security/selinux/ > security/selinux/ss/policydb.h:#define OCON_FS 1 /* > unlabeled file systems */ > security/selinux/ss/policydb.c: if (i == OCON_ISID || i == OCON_FS || > security/selinux/ss/policydb.c: case OCON_FS: > security/selinux/ss/policydb.c: case OCON_FS: > > OCON_FS is only used while parsing a policy and on cleanup, but there > is no actual usage, e.g. for OCON_FSUSE: > > security/selinux/ss/services.c: c = policydb->ocontexts[OCON_FSUSE]; > > So for me fscon is not used at all. > Your assessment is better than mine - thanks for digging deeper. What a shame that code is in-kernel then since it does nothing and there was likely never a policy that had these. Karl >> Good catch though! >> >> Karl >> >> 1. https://media.defense.gov/2021/Jul/29/2002815735/-1/-1/0/SELINUX-SECURITY-POLICY-CONFIGURATION-REPORT.PDF >> 2. https://github.com/torvalds/linux/blob/master/security/selinux/ss/policydb.h#L228 >> >> On Thu, Jun 30, 2022 at 5:05 PM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote: >>> >>> Hello, >>> >>> While studying some malloc calls in libsepol and checkpolicy, I >>> stumbled upon function define_fs_context(), which allocates a >>> fixed-size buffer in >>> https://github.com/SELinuxProject/selinux/blob/956bda08f6183078f13b70f6aa27d0529a3ec20a/checkpolicy/policy_define.c#L4631-L4637 >>> >>> newc->u.name = (char *)malloc(6); >>> if (!newc->u.name) { >>> yyerror("out of memory"); >>> free(newc); >>> return -1; >>> } >>> sprintf(newc->u.name, "%02x:%02x", major, minor); >>> >>> As major and minor are unsigned int (so 32-bit integers) without any >>> value checking, there seems to be a possible heap buffer overflow >>> issue. This function is called when parsing a fscon statement in a >>> "base" policy. So I copied tmp/base.conf from a build of the Reference >>> Policy, added "fscon 1000 1000 system_u:object_r:unlabeled_t >>> system_u:object_r:unlabeled_t" right after "sid security >>> system_u:object_r:security_t" (the order of the statements matters), >>> and ran: >>> >>> $ checkpolicy -o test.pol base.conf >>> *** buffer overflow detected ***: terminated >>> Aborted (core dumped) >>> >>> For whatever it's worth, the stack trace of this abort tells that the >>> buffer overflow occurs in a call to __sprintf_chk(): my gcc compiler >>> seems to be "smart enough" to find out that the size of newc->u.name >>> was 6, and it replaced sprintf() with __sprintf_chk() to ensure that >>> the buffer was not written past its bounds. >>> >>> Now, I can submit a patch to fix this issue, for example by replacing >>> malloc()+sprintf() with asprintf() and by checking that major and >>> minor are below 256. But before doing so, I was wondering: what is >>> this fscon syntax? I have never encountered it, did not find any >>> policy using it, and I am wondering whether we could instead drop its >>> support and remove function define_fs_context() from checkpolicy. >>> >>> Thanks, >>> Nicolas >>>
