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.
> 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
> >
