Commit Graph

731 Commits

Author SHA1 Message Date
Nicolas Iooss
fd705df050 libsepol/cil: do not override previous results of __cil_verify_classperms
When __cil_verify_map_class() verifies a classpermission, it calls
__verify_map_perm_classperms() on each item. If the first item reports a
failure and the next one succeeds, the failure is overwritten in
map_args->rc. This is a bug which causes a NULL pointer dereference in
the CIL compiler when compiling the following policy:

    (sid SID)
    (sidorder (SID))

    (class CLASS (PERM1))
    (classorder (CLASS))

    (classpermission CLSPERM)
    (classpermissionset CLSPERM (CLASS (PERM1)))
    (classmap files (CLAMAPxx x))
    (classmapping files CLAMAPxx CLSPERM)

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=30286

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-07-06 10:27:24 -04:00
James Carter
a0914acf2a
libsepol/cil: Provide option to allow qualified names in declarations
Qualified names have "dots" in them. They are generated when a CIL
policy is compiled and come from declarations in blocks. If a kernel
policy is decompiled into a CIL policy, the resulting policy could
have declarations that use qualified names. Compiling this policy would
result in an error because "dots" in declarations are not allowed.

Qualified names in a policy are normally used to refer to the name of
identifiers, blocks, macros, or optionals that are declared in a
different block (that is not a parent). Name resolution is based on
splitting a name based on the "dots", searching the parents up to the
global namespace for the first block using the first part of the name,
using the second part of the name to lookup the next block using the
first block's symbol tables, looking up the third block in the second's
symbol tables, and so on.

To allow the option of using qualified names in declarations:

1) Create a field in the struct cil_db called "qualified_names" which
is set to CIL_TRUE when qualified names are to be used. This field is
checked in cil_verify_name() and "dots" are allowed if qualified names
are being allowed.

2) Only allow the direct lookup of the whole name in the global symbol
table. This means that blocks, blockinherits, blockabstracts, and in-
statements cannot be allowed. Use the "qualified_names" field of the
cil_db to know when using one of these should result in an error.

3) Create the function cil_set_qualified_names() that is used to set
the "qualified_names" field. Export the function in libsepol.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-07-03 16:00:26 +02:00
Nicolas Iooss
af75f64194
libsepol/cil: make array cil_sym_sizes const
The values of this table are never modified.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-06-30 21:06:07 +02:00
James Carter
4ff514a33e
libsepol/cil: Only reset AST if optional has a declaration
When disabling optionals, the AST needs to be reset only if one
of the optional blocks being disabled contains a declaration.

Call the function cil_tree_subtree_has_decl() for each optional
block being disabled and only reset the AST if one of them has
a declaration in it.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-30 21:05:38 +02:00
James Carter
20271849d5
libsepol/cil: Add function to determine if a subtree has a declaration
Create the function cil_tree_subtree_has_decl() that returns CIL_TRUE
if the subtree has a declaration in it and CIL_FALSE otherwise.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-30 21:05:35 +02:00
James Carter
37863b0b14 libsepol/cil: Improve degenerate inheritance check
The commit 74d00a8dec (libsepol/cil:
Detect degenerate inheritance and exit with an error) detects the
use of inheritance (mostly by the secilc-fuzzer and not in any real
policies) that results in the exponential growth of the policy through
the copying of blocks that takes place with inheritance in CIL.
Unfortunately, the check takes place during the pass when all the
blocks are being copied, so it is possible to consume all of a system's
memory before an error is produced.

The new check happens in two parts. First, a check is made while the
block inheritance is being linked to the block it will inherit. In
this check, all of the parent nodes of the inheritance rule up to the
root node are checked and if enough of these blocks are being inherited
(>= CIL_DEGENERATE_INHERITANCE_DEPTH), then a flag is set for a more
in-depth check after the pass. This in-depth check will determine the
number of potential inheritances that will occur when resolving the
all of the inheritance rules. If this value is greater than
CIL_DEGENERATE_INHERITANCE_GROWTH * the original number of inheritance
rules and greater than CIL_DEGENERATE_INHERITANCE_MINIMUM (which is
set to 0x1 << CIL_DEGENERATE_INHERITANCE_DEPTH), then degenerate
inheritance is determined to have occurred and an error result will
be returned.

Since the potential number of inheritances can quickly be an extremely
large number, the count of potential inheritances is aborted as soon
as the threshold for degenerate inheritance has been exceeded.

Normal policies should rarely, if ever, have the in-depth check occur.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-06-24 10:23:38 -04:00
James Carter
36e494573d libsepol/cil: Reduce the initial symtab sizes for blocks
It is possible to create bad behaving policy that can consume all
of a system's memory (one way is through the use of inheritance).
Analyzing these policies shows that most of the memory usage is for
the block symtabs.

Most of the nineteen symtabs will most likely never be used, so give
these symtabs an initial size of 1. The others are given more
appropriate sizes.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-06-24 10:23:34 -04:00
James Carter
f33745a22b libsepol/cil: Check for empty list when marking neverallow attributes
When marking a type attribute as used in a neverallow (to help determine
whether or not it should be expanded), check if the attribute's expression
list is empty (no attributes are associated with it) before iterating
over the list.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-06-24 10:23:28 -04:00
James Carter
ac8b35d910 libsepol/cil: Fix syntax checking of defaultrange rule
When "glblub" was added as a default for the defaultrange rule, the
syntax array was updated because the "glblub" default does not need
to specify a range of "low", "high", or "low-high". Unfortunately,
additional checking was not added for the "source" and "target"
defaults to make sure they specified a range. This means that using
the "source" or "target" defaults without specifying the range will
result in a segfault.

When the "source" or "target" defaults are used, check that the rule
specifies a range as well.

This bug was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-06-24 10:23:04 -04:00
James Carter
c28525a26f libsepol/cil: Properly check for loops in sets
Commit 61fbdce666 (ibsepol/cil: Check
for self-referential loops in sets) added checks for self-referential
loops in user, role, type, and category sets. Unfortunately, this
check ends up in an infinite loop if the set with the self-referential
loop is used in a different set that is checked before the bad set.

The problem with the old check is that only the initial datum is used
for the check. Instead, use a stack to track all of the set datums
that are currently involved as the check is made. A self-referential
loop occurs if a duplicate datum is found for any of the datums in the
stack.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-06-24 10:03:51 -04:00
James Carter
67a8dc8117 libsepol/cil: Allow duplicate optional blocks in most cases
The commit d155b410d4 (libsepol/cil:
Check for duplicate blocks, optionals, and macros) fixed a bug
that allowed duplicate blocks, optionals, and macros with the same
name in the same namespace. For blocks and macros, a duplicate
is always a problem, but optional block names are only used for
in-statement resolution. If no in-statement refers to an optional
block, then it does not matter if more than one with same name
exists.

One easy way to generate multiple optional blocks with the same
name declaration is to call a macro with an optional block multiple
times in the same namespace.

As an example, here is a portion of CIL policy
  (macro m1 ((type t))
    (optional op1
      (allow t self (CLASS (PERM)))
    )
  )
  (type t1)
  (call m1 (t1))
  (type t2)
  (call m1 (t2))
This will result in two optional blocks with the name op1.

There are three parts to allowing multiple optional blocks with
the same name declaration.

1) Track an optional block's enabled status in a different way.

   One hinderance to allowing multiple optional blocks with the same
   name declaration is that they cannot share the same datum. This is
   because the datum is used to get the struct cil_optional which has
   the enabled field and each block's enabled status is independent of
   the others.

   Remove the enabled field from struct cil_optional, so it only contains
   the datum. Use a stack to track which optional blocks are being
   disabled, so they can be deleted in the right order.

2) Allow multiple declarations of optional blocks.

   Update cil_allow_multiple_decls() so that a flavor of CIL_OPTIONAL
   will return CIL_TRUE. Also remove the check in cil_copy_optional().

3) Check if an in-statement refers to an optional with multiple
   declarations and exit with an error if it does.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-06-24 10:02:26 -04:00
Christian Göttsche
9fb8df7f16 libsepol: declare read-only arrays const
Make it more apparent that those data does not change and enforce it.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:41:30 -04:00
Christian Göttsche
4572bf254a libsepol: declare file local variable static
Clang issues:

    module_to_cil.c:65:7: warning: no previous extern declaration for non-static variable 'out_file' [-Wmissing-variable-declarations]
    FILE *out_file;
          ^

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:41:24 -04:00
Christian Göttsche
4fbc018a27 libsepol: drop unnecessary casts
`hashtab_search()` does take `const_hashtab_key_t` as second parameter,
which is a typedef for `const char *`.
Drop the unnecessary and const-violating cast.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:41:17 -04:00
Christian Göttsche
811185648a libsepol: drop repeated semicolons
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:41:12 -04:00
Christian Göttsche
5324a9ab1b libsepol/cil: avoid using maybe uninitialized variables
Initialize variables, as they are set after goto statements, which jump
to cleanup code using them.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:41:06 -04:00
Christian Göttsche
2723b8ec2a libsepol/cil: drop unnecessary casts
`const_hashtab_key_t` is a typedef of `const char *`, so these casts are
not needed.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:41:01 -04:00
Christian Göttsche
0bb89514eb libsepol/cil: drop dead store
../cil/src/cil_binary.c:2230:24: warning: Value stored to 'cb_node' during its initialization is never read [deadcode.DeadStores]
        struct cil_tree_node *cb_node = node->cl_head;
                              ^~~~~~~   ~~~~~~~~~~~~~

Found by clang-analyzer

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:40:55 -04:00
Christian Göttsche
261b655ac2 libsepol/cil: drop extra semicolon
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:40:49 -04:00
Christian Göttsche
de3b96a158 libsepol/cil: silence cast warning
../cil/src/cil_write_ast.c:86:32: error: cast to smaller integer type 'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
                        enum cil_flavor op_flavor = (enum cil_flavor)curr->data;
                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../cil/src/cil_write_ast.c:130:37: error: cast to smaller integer type 'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
                        enum cil_flavor operand_flavor = (enum cil_flavor)curr->data;
                                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Silence this warning by casting the pointer to an integer the cast to
enum cil_flavor.

See 32f8ed3d6b ("libsepol/cil: introduce intermediate cast to silence -Wvoid-pointer-to-enum-cast")

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:40:40 -04:00
Christian Göttsche
1076a07288 libsepol: remove dead stores
Found by Infer

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:40:35 -04:00
Christian Göttsche
19a6ebfa89 libsepol: do not allocate memory of size 0
In case cats_ebitmap_len() returns 0, do not allocate but quit.

Found by clang-analyzer

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:40:29 -04:00
Christian Göttsche
8eec1bb502 libsepol: mark read-only parameters of type_set_ interfaces const
Make it more obvious which parameters are read-only and not being
modified and allow callers to pass const pointers.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:40:24 -04:00
Christian Göttsche
390ec54d27 libsepol: mark read-only parameters of ebitmap interfaces const
Make it more obvious which parameters are read-only and not being
modified and allow callers to pass const pointers.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:40:18 -04:00
Christian Göttsche
a53a845b76 libsepol: remove dead stores
conditional.c:391:4: warning: Value stored to 'i' is never read [deadcode.DeadStores]
                        i = 0;
                        ^   ~
conditional.c:718:2: warning: Value stored to 'len' is never read [deadcode.DeadStores]
        len = 0;
        ^     ~
conditional.c:772:2: warning: Value stored to 'len' is never read [deadcode.DeadStores]
        len = 0;
        ^     ~

services.c:89:10: warning: Value stored to 'new_stack' during its initialization is never read [deadcode.DeadStores]
                char **new_stack = stack;
                       ^~~~~~~~~   ~~~~~

services.c:440:11: warning: Value stored to 'new_expr_list' during its initialization is never read [deadcode.DeadStores]
                        char **new_expr_list = expr_list;
                               ^~~~~~~~~~~~~   ~~~~~~~~~

../cil/src/cil_binary.c:2230:24: warning: Value stored to 'cb_node' during its initialization is never read [deadcode.DeadStores]
        struct cil_tree_node *cb_node = node->cl_head;
                              ^~~~~~~   ~~~~~~~~~~~~~

Found by clang-analyzer

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:40:10 -04:00
Christian Göttsche
852c4398a9 libsepol/cil: follow declaration-after-statement
Follow the project style of no declaration after statement.

Found by the gcc warning -Wdeclaration-after-statement

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:40:03 -04:00
Christian Göttsche
8f50b45320 libsepol: follow declaration-after-statement
Follow the project style of no declaration after statement.

Found by the gcc warning -Wdeclaration-after-statement

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:39:56 -04:00
Christian Göttsche
1537ea8412 libsepol: avoid unsigned integer overflow
Unsigned integer overflow is well-defined and not undefined behavior.
But it is still useful to enable undefined behavior sanitizer checks on
unsigned arithmetic to detect possible issues on counters or variables
with similar purpose.

Use a spaceship operator like comparison instead of subtraction.

Modern compilers will generate a single comparison instruction instead
of actually perform the subtraction.

policydb.c:826:17: runtime error: unsigned integer overflow: 24 - 1699 cannot be represented in type 'unsigned int'

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:39:48 -04:00
Christian Göttsche
42f3d7cceb libsepol: remove unused functions
The functions `role_set_get_role`, `sepol_validate_transition` and
`sepol_sidtab_remove` seem to be unused since the initial import.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:39:41 -04:00
Christian Göttsche
9ec061b61c libsepol: resolve missing prototypes
Declare the functions as static or include the corresponding header
file.

assertion.c:294:5: error: no previous prototype for function 'report_assertion_failures' [-Werror,-Wmissing-prototypes]
int report_assertion_failures(sepol_handle_t *handle, policydb_t *p, avrule_t *avrule)
    ^

context.c:23:5: error: no previous prototype for function 'sepol_check_context' [-Werror,-Wmissing-prototypes]
int sepol_check_context(const char *context)
    ^

expand.c:3377:5: error: no previous prototype for function 'expand_cond_av_node' [-Werror,-Wmissing-prototypes]
int expand_cond_av_node(policydb_t * p,
    ^

policydb.c:638:6: error: no previous prototype for function 'role_trans_rule_destroy' [-Werror,-Wmissing-prototypes]
void role_trans_rule_destroy(role_trans_rule_t * x)
     ^

policydb.c:1169:5: error: no previous prototype for function 'policydb_index_decls' [-Werror,-Wmissing-prototypes]
int policydb_index_decls(sepol_handle_t * handle, policydb_t * p)
    ^

policydb.c:1429:6: error: no previous prototype for function 'ocontext_selinux_free' [-Werror,-Wmissing-prototypes]
void ocontext_selinux_free(ocontext_t **ocontexts)
     ^

policydb.c:1451:6: error: no previous prototype for function 'ocontext_xen_free' [-Werror,-Wmissing-prototypes]
void ocontext_xen_free(ocontext_t **ocontexts)
     ^

policydb.c:1750:5: error: no previous prototype for function 'type_set_or' [-Werror,-Wmissing-prototypes]
int type_set_or(type_set_t * dst, type_set_t * a, type_set_t * b)
    ^

policydb.c:2524:5: error: no previous prototype for function 'role_trans_read' [-Werror,-Wmissing-prototypes]
int role_trans_read(policydb_t *p, struct policy_file *fp)
    ^

policydb.c:2567:5: error: no previous prototype for function 'role_allow_read' [-Werror,-Wmissing-prototypes]
int role_allow_read(role_allow_t ** r, struct policy_file *fp)
    ^

policydb.c:2842:5: error: no previous prototype for function 'filename_trans_read' [-Werror,-Wmissing-prototypes]
int filename_trans_read(policydb_t *p, struct policy_file *fp)
    ^

services.c:1027:5: error: no previous prototype for function 'sepol_validate_transition' [-Werror,-Wmissing-prototypes]
int sepol_validate_transition(sepol_security_id_t oldsid,
    ^

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:39:22 -04:00
Christian Göttsche
2cb6bacddc libsepol: fix typos
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-06-24 09:38:54 -04:00
James Carter
ce1025bf9c libsepol: Quote paths when generating policy.conf from binary policy
Christian Göttsche <cgzones@googlemail.com> submitted a similar patch
to quote paths when generating CIL policy from a binary policy.

Since genfscon and devicetreecon rules have paths which are allowed
to contain spaces, always quote the path when writing out these rules.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
2021-06-22 09:34:26 -04:00
James Carter
982ec302b6 libsepol/cil: Account for anonymous category sets in an expression
It is possible for anonymous category sets to be in a category
expression if the expression has a macro parameter in it.
Unfortunately, anonymous category sets are not looked for when
resolving category expressions and a segfault will occur during
later processing if there was one.

As an example, consider the following portion of a policy.
  (macro m1 ((categoryset cs))
    (userlevel USER (s0 (cs)))
  )
  (call m1 ((c0 c1)))
This policy will cause a segault, because the categoryset datum
for the parameter cs is not seen as a categoryset and is treated
as a plain category.

When resolving an expression, check whether or not the datum that
is found is actually an anonymous category set associated with a
macro parameter. If it is, then resolve the category set if it
has not already been resolved and treat its categories as a sub
expression.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-06-22 09:32:57 -04:00
James Carter
9ac9d2dab4 libsepol/cil: Fix anonymous IP address call arguments
A named IP address (using an ipaddr rule) could be passed as an
argument, but trying to pass an actual IP address caused an error.

As an exmample, consider the following portion of a policy.
  (macro m4 ((ipaddr ip)(ipaddr nm))
    (nodecon ip nm (USER ROLE TYPE ((s0) (s0))))
  )
  (ipaddr nm1 255.255.255.0)
  (ipaddr ip1 1.2.3.4)
  (call m4 (ip1 nm1)) ; This works
  (call m4 (1.2.3.4 255.255.255.0)) ; This doesn't

Allow actual IP addresses to be passed as a call argument. Now the
second call works as well.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-06-22 09:32:23 -04:00
Christian Göttsche
644c5bbbc4 libsepol: quote paths in CIL conversion
When generating CIL policy from kernel or module policy quote paths,
which are allowed to contain spaces, in the statements `genfscon` and
`devicetreecon`.

Reported by LuK1337 while building policy for Android via IRC.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-06-14 09:35:03 -04:00
James Carter
d8b90f8ad1 libsepol/cil: Resolve anonymous levels only once
Anonymous levels can be passed as call arguments and they can
appear in anonymous levelranges as well.

Anonymous call arguments are resolved when they are used in a rule.
If more than one rule uses the anonymous level, then a memory leak
will occur when a new list for the category datum expression is
created without destroying the old one.

When resolving a level, check if the sensitivity datum has already
been resolved. If it has, then the categories have been as well.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:49 -04:00
James Carter
73d991abdc libsepol/cil: Pointers to datums should be set to NULL when resetting
Set the pointer to the sensitivity in levels, the pointers to the low
and high levels in levelranges, the pointer to the level in userlevels,
the pointer to the range in userranges, and the pointers to contexts
in ocontexts to NULL.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:47 -04:00
James Carter
a8dcf4d57b libsepol/cil: Resolve anonymous class permission sets only once
Anonymous class permission sets can be passed as call arguments.
Anonymous call arguments are resolved when they are used in a
rule. [This is because all the information might not be present
(like common permissions being added to a class) when the call
itself is resolved.] If there is more than one rule using an
anonymous class permission set, then a memory leak will occur
when a new list for the permission datum expression is created
without destroying the old one.

When resolving the class and permissions, check if the class has
already been resolved. If it has, then the permissions have been
as well.

This bug was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:44 -04:00
James Carter
69fc31d1fb libsepol/cil: Limit the number of open parenthesis allowed
When parsing a CIL policy, the number of open parenthesis is tracked
to verify that each has a matching close parenthesis. If there are
too many open parenthesis, a stack overflow could occur during later
processing.

Exit with an error if the number of open parenthesis exceeds 4096
(which should be enough for any policy.)

This bug was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:42 -04:00
James Carter
29d6a3ee4a libsepol/cil: Destroy the permission nodes when exiting with an error
When exiting with an error because a class or common has too many
permissions, destroy the permission nodes.

This bug was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:40 -04:00
James Carter
5661efd459 libsepol/cil: Handle disabled optional blocks in earlier passes
A failed tunable resolution in a tunableif can cause an optional
to be disabled before the CIL_PASS_CALL1 phase. If this occurs, the
optional block and its subtree should be destroyed, but no reset
will be required since tunables are not allowed inside an optional
block.

Anytime there are optional blocks in the disabled_optionals list
(changed == 1), destroy the optional block and its subtree even if
in a pass before CIL_PASS_CALL1.

This bug was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:38 -04:00
James Carter
aa8ac8ffaf libsepol/cil: Do not resolve arguments to declarations in the call
Lorenzo Ceragioli <lorenzo.ceragioli@phd.unipi.it> noted that the
following policy:
  (type a)
  (block A
    (macro m ((type x))
      (type a)
      (allow x x (file (read))))
  )
  (block B
    (call A.m(a))
  )
results in the allow rule (allow B.a B.a (file(read))). This makes
no sense because the "a" being passed as an argument has to be the
global "a" and not the "a" defined in the macro.

This behavior occurs because the call arguments are resolved AFTER
the macro body has been copied and the declaration of "a" in the
macro has been added to block B's namespace, so this is the "a"
that the call argument resolves to, rather than the one in the
global namespace.

When resolving call arguments, check if the datum found belongs to
a declaration in the call. If it does, then remove the datum from
the symbol table, re-resolve the argument, and add the datum back
into the symbol table.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:19 -04:00
James Carter
bccec36a76 libsepo/cil: Refactor macro call resolution
Rename cil_resolve_call1() as cil resolve_call() and rename
cil_resolve_call2() as cil_resolve_call_args() to make it clearer
what is being done in each function.

Move code to build call arguments out of cil_resolve_call() and into
the new function called cil_build_call_args() so that the logic of
cil_resolve_call() can be seen.

Exit cil_resolve_call() immediately if the call has already been
copied.

In __cil_resolve_ast_node(), only resolve calls outside of macros.
This results in more calls to cil_copy_ast(), but slightly less
rules copied overall (since no rules are copied into a macro). This
also means that the CIL_PASS_MACRO pass is not needed and can be
eliminated.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:17 -04:00
James Carter
a1952af7c0 libsepol/cil: Do not add NULL node when inserting key into symtab
Allow inserting a key without providing a node.

This will make it easier to properly resolve call arguments where
a key might need to be temporarily removed to search for a datum
that is not declared within the call. Since the node is already
in the node list, re-inserting the key without this option would
add another link to the node and cause problems.

Also, do not add the node to the datum's node list if the result
of the call to hashtab_insert() is SEPOL_EEXIST because the datum
is a duplicate and will be destroyed.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:14 -04:00
James Carter
788d40b0e6 libsepol/cil: Make name resolution in macros work as documented
The CIL Reference Guide specifies how name resolution is suppose
to work within an expanded macro.
  1. Items defined inside the macro
  2. Items passed into the macro as arguments
  3. Items defined in the same namespace of the macro
  4. Items defined in the caller's namespace
  5. Items defined in the global namespace

But Lorenzo Ceragioli <lorenzo.ceragioli@phd.unipi.it> found
that the first step is not done.

So the following policy:
  (block A
    (type a)
    (macro m ()
      (type a)
      (allow a self (CLASS (PERM)))
    )
  )
  (block B
    (call A.m)
  )
will result in:
  (allow A.a self (CLASS (PERM)))
instead of the expected:
  (allow B.a self (CLASS (PERM)))

Now when an expanded call is found, the macro's namespace is
checked first. If the name is found, then the name was declared
in the macro and it is declared in the expanded call, so only the
namespace of the call up to and including the global namespace
will be searched. If the name is not found in the macro's namespace
then name resolution continues with steps 2-5 above.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-06-04 10:23:09 -04:00
James Carter
0d6e95cfb2 libsepol/cil: Fix name resolution involving inherited blocks
When resolving a name in a block that has been inherited. First,
a search is done in the parent namespaces (if any) of the
blockinherit rule with the exception of the global namespace. If
the name is not found, then a search is done in the namespaces of
the original block (starting with that block's namespace)  with
the exception of the global namespace. Finally, if it still has
not been found, the global namespace is searched.

This does not work if a declaration is in the block being
inherited.

For example:
  (block b
    (typeattribute a)
    (allow a self (CLASS (PERM)))
  )
  (blockinherit b)

This will result in a policy with the following identical allow
rules:
  (allow b.a self (CLASS (PERM)))
  (allow b.a self (CLASS (PERM)))
rather than the expected:
  (allow b.a self (CLASS (PERM)))
  (allow a self (CLASS (PERM)))
This is because when the typeattribute is copied while resolving
the inheritance, the new datum is added to the global namespace
and, since that is searched last, the typeattribute in block b is
found first.

This behavior means that no declaration that is inherited into the
global namespace will actually be used.

Instead, if the name is not found in the parent namespaces (if any)
where the blockinherit is located with the exception of the global
namespace, start the next search in the namespace of the parent of
the original block (instead of the original block itself). Now if
a declaration is inherited from the original block, the new
declaration will be used. This behavior seems to be the originally
intended behavior because there is a comment in the code that says,
"Continue search in original block's parent".

This issue was found by secilc-fuzzer. If the original block
is made to be abstract, then the type attribute declaration
in the original block is not in the policy and a segfault
occurs when creating the binary because the copied allow rule
refers to a non-existent type attribute.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-05-31 12:38:15 +02:00
James Carter
61fbdce666 libsepol/cil: Check for self-referential loops in sets
The secilc-fuzzer found a self-referential loop using category sets.
Any set declaration in CIL that allows sets in it is susceptible to
the creation of a self-referential loop. There is a check, but only
for the name of the set being declared being used in the set
declaration.

Check for self-refential loops in user, role, and type attributes
and in category sets. Since all of the sets need to be declared,
this check has to be done when verifying the CIL db before doing
the post phase.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-05-13 10:55:34 -04:00
James Carter
d9433692c7 libsepol/cil: Return an error if a call argument fails to resolve
Return an error if a call argument fails to resolve so that
the resolution phase stops and returns an error.

This problem was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-05-04 15:55:32 -04:00
James Carter
d438b6cfb3 libsepol/cil: Check datum in ordered list for expected flavor
The secilc-fuzzer found an out of bounds memory access occurs
when building the binary policy if a map class is included in a
classorder statement.

The order statements in CIL (sidorder, classorder, categoryorder,
and sensitivityorder) are used to specify an ordering for sids,
classes, categories, and sensitivities. When the order statments
are resolved and merged, only in the case of the category order
list is the datum resolved checked to see if it is the expected
flavor.

When resolving the sid, class, and sensitivity order statements,
check that each name resolved to a datum of the expected flavor
and return an error if it does not.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-05-04 15:55:12 -04:00
James Carter
74d00a8dec libsepol/cil: Detect degenerate inheritance and exit with an error
A CIL policy with inheritance of the form
...
(blockinherit ba)
(block ba
  (block b1
    (blockinherit bb)
  )
  (block bb
    (block b2
      (blockinherit bc)
    )
    (block bc
      (block b3
        (blockinherit bd)
      )
      (block bd
        (block b4
          (blockinherit be)
        )
        (block be
        ...
will require creating 2^depth copies of the block at the bottom of
the inheritance chain. This pattern can quickly consume all the
memory of the system compiling this policy.

The depth of the inheritance chain can be found be walking the
tree up through the parents and noting how many of the parent
blocks have been inherited. The number of times a block will be
copied is found by counting the list of nodes in the "bi_nodes"
list of the block. To minimize legitimate policies from being
falsely detected as being degenerate, both the depth and breadth
(number of copies) are checked and an error is given only if both
exceed the limits (depth >= 12 and breadth >= 4096).

This problem was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-05-04 15:55:06 -04:00