Re: [PATCH] libsepol/cil: Give an error when constraint expressions exceed max depth

[Dominick Grift <dominick.grift@defensec.nl>]

Stephen Smalley <stephen.smalley.work@gmail.com> writes:

> On Tue, Sep 8, 2020 at 9:46 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
>>
>> On Fri, Sep 4, 2020 at 8:49 AM Stephen Smalley
>> <stephen.smalley.work@gmail.com> wrote:
>> >
>> > On Thu, Sep 3, 2020 at 2:19 PM James Carter <jwcart2@gmail.com> wrote:
>> > >
>> > > CIL was not correctly determining the depth of constraint expressions
>> > > which prevented it from giving an error when the max depth was exceeded.
>> > > This allowed invalid policy binaries with constraint expressions exceeding
>> > > the max depth to be created.
>> > >
>> > > Correctly calculate the depth of constraint expressions when building
>> > > the AST and give an error when the max depth is exceeded.
>> > >
>> > > Reported-by: Jonathan Hettwer <j2468h@gmail.com>
>> > > Signed-off-by: James Carter <jwcart2@gmail.com>
>> >
>> > The fix for conditional boolean expression depth checking can be a
>> > separate patch.  For this one,
>> >
>> > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>>
>> Actually, this breaks selinux-testsuite. Will have to look into why.
>> /usr/sbin/semodule -i test_policy/test_policy.pp test_mlsconstrain.cil
>> test_overlay_defaultrange.cil test_add_levels.cil test_glblub.cil
>> Max depth of 4 exceeded for constraint expression
>> Bad expression tree for constraint
>> Bad constrain declaration at
>> /var/lib/selinux/targeted/tmp/modules/100/base/cil:919
>
> Here is the failing cil module:
> $ cat policy/test_mlsconstrain.cil
> (mlsconstrain (peer (recv)) (or (dom l1 l2) (and (neq t1
> mcs_constrained_type) (neq t2 mcs_constrained_type))))
> (mlsconstrain (packet (recv)) (or (dom l1 l2) (and (neq t1
> mcs_constrained_type) (neq t2 mcs_constrained_type))))
>
> Maybe an off-by-one in your depth checking?

That looks scary to me. Those constrains are simple compared to some of
the ones I currently successfully use and rely upon:

https://git.defensec.nl/?p=dssp3.git;a=blob;f=policy/constrain/rbacsep.cil;h=935c722167bcf214f286eb339e0793cd94d3edd0;hb=HEAD

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift