Commit graph

3144 commits

Author SHA1 Message Date
Petr Lautrbach
db3863d551 libselinux/semodule: Improve extracting message
The code doesn't check the default priority, it just looks for the
highest.

Fixes:

    # semodule -E testmodule
    Module 'testmodule' does not exist at the default priority '400'. Extracting at highest existing priority '400'.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-10-06 12:04:03 +02:00
James Carter
85d0a069ce libsepol/cil: Do not skip macros when resolving until later passes
Since only tunableifs need to be resolved in a macro before the macro
is copied for each call, macros were being skipped after resolving
tunableifs. Statments not allowed to be in macros would be found during
the pass that resolved tunableifs. Unfortunately, in-statments are
resolved after tunableifs and they can be used to add statements to
macros that are not allowed.

Instead, do not skip macros until after the pass that resolves in-
statements that are to be resolved after block inheritance. This
allows blocks, blockinherits, blockabstracts, and macros that were
added by an in-statement to be found and an error reported.

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-10-06 12:03:48 +02:00
James Carter
4b2e2a248e libsepol/cil: Limit the amount of reporting for bounds failures
Type bounds are checked when creating the CIL binary using libsepol
functions on the binary policy db. The bad rule is reported and, to
provide better error reporting, a search is made for matching rules
in the CIL policy. These matching rules as well as their parents are
written out with their locations to make it easier to find the rules
that violate the type bounds.

It is possible to craft CIL policies where there are many rules
that violate a bounds check each with many matching rules as well.
This can make the error messages very difficult to deal with. For
example, if there are 100 rules in the binary policy db that violate
a type bounds and each of these rules has 100 matches, then 10,000
matching rules along with their parents will be written out as part
of the error message.

Limit the error reporting to two rules for each type bounds violation
along with two matches for each of those rules.

This problem was found with the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-10-06 12:03:14 +02:00
Nicolas Iooss
b2891256cb GitHub Actions: do not use macOS latest runner for now
GitHub is currently migrating its macos-latest runner to macOS 11:
https://github.com/actions/virtual-environments/issues/4060

Unfortunately, installing VirtualBox and Vagrant on this new version of
macOS is not easy, and the current macos-11 runner does not support
running virtual machines using Vagrant. This issue is being fixed on
https://github.com/actions/virtual-environments/pull/4010 .

Until this Pull Request is merged, use macos-10.15 runner instead of
macos-latest, to continue using Vagrant to run the SELinux testsuite in
a virtual machine.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
2021-10-04 09:50:46 -04:00
Nicolas Iooss
60bde947d6 README: update continuous integration badges
The CI now uses GitHub Actions to run tests and the SELinux testsuite in
a virtual machine. Replace the Travis CI badge with the ones for these
workflows.

Fixes: https://github.com/SELinuxProject/selinux/issues/299
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
2021-10-04 09:49:46 -04:00
Christian Göttsche
eac7fb001a checkpolicy: delay down-cast to avoid align warning
Delay the down-cast from hashtab_datum_t, alias void*, to the actual
type once its kind has been determined.

    module_compiler.c:174:19: warning: cast from 'symtab_datum_t *' (aka 'struct symtab_datum *') to 'level_datum_t *' (aka 'struct level_datum *') increases required alignment from 4 to 8 [-Wcast-align]
                            *dest_value = ((level_datum_t *)s)->level->sens;
                                           ^~~~~~~~~~~~~~~~~~

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:25:18 -04:00
Christian Göttsche
44ed3074bb checkpolicy: drop incorrect cast
The function require_symbol takes the type hashtab_datum_t (alias void*)
as third argument. Do not cast to hashtab_datum_t* alias void**. Since
explicit casting to void* is unnecessary, drop the casts.

    module_compiler.c:1002:36: warning: cast from 'cond_bool_datum_t *' (aka 'struct cond_bool_datum *') to 'hashtab_datum_t *' (aka 'void **') increases required alignment from 4 to 8 [-Wcast-align]
                require_symbol(SYM_BOOLS, id, (hashtab_datum_t *) booldatum,
                                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    module_compiler.c:1092:40: warning: cast from 'cat_datum_t *' (aka 'struct cat_datum *') to 'hashtab_datum_t *' (aka 'void **') increases required alignment from 4 to 8 [-Wcast-align]
            retval = require_symbol(SYM_CATS, id, (hashtab_datum_t *) cat,
                                                  ^~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:25:15 -04:00
Christian Göttsche
7a1c7e7a82 checkpolicy: update documentation
Add missing command-line arguments to synopsis and highlight mentions of
other tools in man pages.

Add missing space between arguments in help message.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:25:12 -04:00
Christian Göttsche
e32809155b checkpolicy: print reason of fopen failure
Print the reason why opening a source policy file failed, e.g:

    checkpolicy:  unable to open policy.conf:  No such file or directory

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:25:10 -04:00
Christian Göttsche
3d27e5a410 checkpolicy: policy_define: cleanup declarations
The variable curfile is nowhere used.

Static functions do not need to be forward declared if not used before
their definition.

The error buffer errormsg can be a simple scoped variable. Also
vsnprintf(3) always NUL-terminates the buffer, so the whole length can
be passed.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:25:07 -04:00
Christian Göttsche
6c81478456 libsepol/cil: silence clang void-pointer-to-enum-cast warning
Add an intermediate cast to uintptr_t to silence the clang specific
warning about casting a void pointer to an enum.

    ../cil/src/cil_verify.c:1749:28: error: cast to smaller integer type 'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
                                                    enum cil_flavor op = (enum cil_flavor)i->data;
                                                                         ^~~~~~~~~~~~~~~~~~~~~~~~

Similar to 32f8ed3d6b.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:25:05 -04:00
Christian Göttsche
e0968a8640 libsepol: resolve GCC warning about null-dereference
GCC reports a NULL dereference of the return value of stack_peek(). This
function explicitly returns NULL in case of 'stack->pos == -1'.

Error out on NULL returned.

    module_to_cil.c: In function ‘block_to_cil’:
    module_to_cil.c:3357:55: error: potential null pointer dereference [-Werror=null-dereference]
     3357 |         struct list *alias_list = typealias_lists[decl->decl_id];
          |                                                   ~~~~^~~~~~~~~

There are more occurrences of unconditionally dereferencing the return
value of stack_peek(), but the callers should ensure a valid stack, so
just silence this single warning.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:24:56 -04:00
Christian Göttsche
a9f4a2cb32 libsepol: use correct cast
The function hashtab_insert takes the type hashtab_datum_t (alias void*)
as third argument. Do not cast to hashtab_datum_t* alias void**. The
casts could be dropped, as explicit casting to void* is unnecessary, but
to fit the overall style of this file keep the casts.

    expand.c:246:41: error: cast from 'perm_datum_t *' (aka 'struct perm_datum *') to 'hashtab_datum_t *' (aka 'void **') increases required alignment from 4 to 8 [-Werror,-Wcast-align]
            ret = hashtab_insert(s->table, new_id, (hashtab_datum_t *) new_perm);
                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:24:53 -04:00
Christian Göttsche
1ba9d982a7 libsepol: ebitmap: mark nodes of const ebitmaps const
Mark pointers to nodes of const ebitmaps also const. C does not enforce
a transitive const-ness, but it clarifies the intent and improves
maintainability.

Follow-up of 390ec54d27

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-10-04 09:24:49 -04:00
Christian Göttsche
b1a3c2030c Correct some typos
Found by codespell

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
2021-09-30 15:42:11 -04:00
Petr Lautrbach
0b833973bf
Update VERSIONs to 3.3-rc2 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-09-22 17:14:25 +02:00
Christian Göttsche
ffa73128ea libselinux/utils: drop requirement to combine compiling and linking
The extra dependency of sefcontext_compile on its object file causes the
compile and link step to be separated.
During the link step the CFLAGS are not passed, which might contain
optimization or sanitizer flags.

Reorder the LDLIBS requirements to avoid the symbol 'pcre_fullinfo'
being unresolvable at link time.

Current behavior:

    gcc-11 **custom CFLAGS** -I../include -D_GNU_SOURCE     -c -o sefcontext_compile.o sefcontext_compile.c
    gcc-11 -L../src  sefcontext_compile.o ../src/regex.o  -lselinux  -lpcre ../src/libselinux.a -lsepol -o sefcontext_compile

Changed:

    gcc-11 **custom CFLAGS** -I../include -D_GNU_SOURCE    -L../src  sefcontext_compile.c  -lselinux  ../src/libselinux.a -lpcre -lsepol -o sefcontext_compile

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-22 12:31:53 +02:00
James Carter
18f8747b28 libsepol/cil: Handle operations in a class mapping when verifying
When checking for circular class permission declarations and a class
mapping is encountered, the class permissions for each map permission
must be checked. An assumption was made that there were no operators
in the class permissions. An operator in the class permissions would
cause a segfault.

Example causing segault:
  (classmap cm1 (mp1))
  (classmapping cm1 mp1 (CLASS (PERM)))
  (classpermission cp1)
  (classpermissionset cp1 (cm1 (all)))

For map class permissions, check each item in the permission list to
see if it is an operator. If it is not, then verify the class
permissions associated with the map permission. If it is an operator
and the operator is "all", then create a list of all permissions for
that map class and verify the class permissions associated with each
map permission. If it is a different operator, then it can be skipped.

This bug was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-22 10:01:19 +02:00
James Carter
34f7a55d7e libsepol/cil: Do not use original type and typeattribute datums
When compiling CIL policy using secilc's "-m" option (which allows
duplicate declarations for types and type attributes), a segfault
will occur if the type or type attribute being copied has already
been declared. This is because a search of the symbol table is made
during the copy and the original datum will be used if one is found.
The original datum will be considered a duplicate when an attempt is
made to add it to the symbol table. The original datum, which is still
in use, will then be destroyed and a segfault will follow soon after
that.

Instead, always create a new datum. When it is added the new datum
will be destroyed if it is a duplicate and duplicate declarations
are allowed.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-22 09:46:20 +02:00
Christian Göttsche
a0a342c37b checkpolicy: free extended permission memory
define_te_avtab_xperms_helper() allocates memory for the avrule, while
define_te_avtab_ioctl() does not transfer any ownership of it.
Free the affected memory.

    Direct leak of 272 byte(s) in 2 object(s) allocated from:
        #0 0x49bb8d in __interceptor_malloc (./checkpolicy/checkmodule+0x49bb8d)
        #1 0x4f379c in define_te_avtab_xperms_helper ./checkpolicy/policy_define.c:2047:24
        #2 0x4f379c in define_te_avtab_extended_perms ./checkpolicy/policy_define.c:2469:6
        #3 0x4cf417 in yyparse ./checkpolicy/policy_parse.y:494:30
        #4 0x4eaf35 in read_source_policy ./checkpolicy/parse_util.c:63:6
        #5 0x50cccd in main ./checkpolicy/checkmodule.c:278:7
        #6 0x7fbfa455ce49 in __libc_start_main csu/../csu/libc-start.c:314:16

    Direct leak of 32 byte(s) in 2 object(s) allocated from:
        #0 0x49bb8d in __interceptor_malloc (./checkpolicy/checkmodule+0x49bb8d)
        #1 0x4f4a38 in avrule_sort_ioctls ./checkpolicy/policy_define.c:1844:12
        #2 0x4f4a38 in avrule_ioctl_ranges ./checkpolicy/policy_define.c:2021:6
        #3 0x4f4a38 in define_te_avtab_ioctl ./checkpolicy/policy_define.c:2399:6
        #4 0x4f4a38 in define_te_avtab_extended_perms ./checkpolicy/policy_define.c:2475:7
        #5 0x4cf417 in yyparse ./checkpolicy/policy_parse.y:494:30
        #6 0x4eaf35 in read_source_policy ./checkpolicy/parse_util.c:63:6
        #7 0x50cccd in main ./checkpolicy/checkmodule.c:278:7
        #8 0x7fbfa455ce49 in __libc_start_main csu/../csu/libc-start.c:314:16

Reported-by: liwugang <liwugang@163.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:42 -04:00
Christian Göttsche
ba18cf0cdf checkpolicy: print warning on source line overflow
In case the source line value overflows or has a too big value in the
source policy print a warning.

    policy_scan.l:273:19: runtime error: implicit conversion from type 'int' of value -2 (32-bit, signed) to type 'unsigned long' changed the value to 18446744073709551614 (64-bit, unsigned)
    policy_scan.l:66:20: runtime error: unsigned integer overflow: 18446744073709551615 + 1 cannot be represented in type 'unsigned long'

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:37 -04:00
Christian Göttsche
15fcc6df66 checkpolicy: error out on parsing too big integers
Error out instead of silently converting too big integer values in
policy sources.

    policy_parse.y:893:41: runtime error: implicit conversion from type 'unsigned long' of value 18446744073709551615 (64-bit, unsigned) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:33 -04:00
Christian Göttsche
c7c582a0ef checkpolicy: avoid implicit conversion
Avoid implicit conversions from signed to unsigned values, found by
UB sanitizers, by using unsigned values in the first place.

    dismod.c:92:42: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:27 -04:00
Christian Göttsche
16d7dde41c checkpolicy: resolve dismod memory leaks
Example leak:

    Indirect leak of 4 byte(s) in 1 object(s) allocated from:
        #0 0x49bacd in __interceptor_malloc (./checkpolicy/test/dismod+0x49bacd)
        #1 0x58ae54 in add_i_to_a ./libsepol/src/util.c:55:21
        #2 0x53ea8e in symtab_insert ./libsepol/src/policydb.c:1729:6
        #3 0x536252 in roles_init ./libsepol/src/policydb.c:772:7
        #4 0x536252 in policydb_init ./libsepol/src/policydb.c:892:7
        #5 0x562ff1 in sepol_policydb_create ./libsepol/src/policydb_public.c:69:6
        #6 0x521a7c in module_package_init ./libsepol/src/module.c:96:6
        #7 0x521a7c in sepol_module_package_create ./libsepol/src/module.c:126:7
        #8 0x4cfb80 in read_policy ./checkpolicy/test/dismod.c:750:7
        #9 0x4cda10 in main ./checkpolicy/test/dismod.c:878:6
        #10 0x7f8538d01e49 in __libc_start_main csu/../csu/libc-start.c:314:16

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:20 -04:00
Christian Göttsche
e6bab7bf45 checkpolicy: add missing function declarations
Ideally they should be declared in the corresponding header file, but
the overall include style in the checkpolicy code is quite messy.
Declare them for now in the source file before defining them to silence
related compiler warnings:

    policy_define.c:84:6: error: no previous prototype for function 'init_parser' [-Werror,-Wmissing-prototypes]
    void init_parser(int pass_number)
         ^
    policy_define.c:93:6: error: no previous prototype for function 'yyerror2' [-Werror,-Wmissing-prototypes]
    void yyerror2(const char *fmt, ...)
         ^

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:09 -04:00
Christian Göttsche
5c376d6db1 checkpolicy: mark file local functions in policy_define static
Also remove the unused function `avrule_ioctl_freeranges()`.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:14:00 -04:00
Christian Göttsche
6388cfe7f3 checkpolicy: mark read-only parameters in module compiler 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>
2021-09-15 10:13:58 -04:00
Christian Göttsche
9eebc4b731 checkpolicy: misc checkpolicy tweaks
Add missing argument in usage message.
Drop redundant includes `optarg` and `optind`, which are declared in
<getopt.h>.
Mark file local functions static.
Drop unused function declaration.

Check closing file streams after writing, which can signal a failed
write or sync to disk and should be checked.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:13:54 -04:00
Christian Göttsche
9696bb5449 checkpolicy: misc checkmodule tweaks
Add missing argument in usage message.
Drop redundant includes `optarg` and `optind`, which are declared in
<getopt.h>.
Use consistent quit style by using `exit(1)`.
Mark read-only options struct const.
Check closing file streams after writing, which can signal a failed
write or sync to disk and should be checked.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:13:51 -04:00
Christian Göttsche
5570c2e394 checkpolicy: enclose macro argument in parentheses
Found by clang-tidy

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:13:48 -04:00
Christian Göttsche
474b271b1b libsepol: free memory after policy validation
Found while running the checkpolicy/test/dispol binary.

    Direct leak of 24 byte(s) in 1 object(s) allocated from:
        #0 0x49bacd in __interceptor_malloc (./checkpolicy/test/dispol+0x49bacd)
        #1 0x5551e1 in ebitmap_set_bit ./libsepol/src/ebitmap.c:326:27
        #2 0x517873 in create_gap_ebitmap ./libsepol/src/policydb_validate.c:23:8
        #3 0x517873 in validate_init ./libsepol/src/policydb_validate.c:34:6
        #4 0x50fa47 in validate_array_init ./libsepol/src/policydb_validate.c:44:6
        #5 0x50fa47 in validate_policydb ./libsepol/src/policydb_validate.c:732:6
        #6 0x4f22df in policydb_read ./libsepol/src/policydb.c:4538:6
        #7 0x4cddb3 in main ./checkpolicy/test/dispol.c:437:8
        #8 0x7f5980e47e49 in __libc_start_main csu/../csu/libc-start.c:314:16

    Indirect leak of 48 byte(s) in 2 object(s) allocated from:
        #0 0x49bacd in __interceptor_malloc (./checkpolicy/test/dispol+0x49bacd)
        #1 0x5551e1 in ebitmap_set_bit ./libsepol/src/ebitmap.c:326:27
        #2 0x517873 in create_gap_ebitmap ./libsepol/src/policydb_validate.c:23:8
        #3 0x517873 in validate_init ./libsepol/src/policydb_validate.c:34:6
        #4 0x50fa47 in validate_array_init ./libsepol/src/policydb_validate.c:44:6
        #5 0x50fa47 in validate_policydb ./libsepol/src/policydb_validate.c:732:6
        #6 0x4f22df in policydb_read ./libsepol/src/policydb.c:4538:6
        #7 0x4cddb3 in main ./checkpolicy/test/dispol.c:437:8
        #8 0x7f5980e47e49 in __libc_start_main csu/../csu/libc-start.c:314:16

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:13:37 -04:00
Christian Göttsche
12e9846205 libsepol: avoid implicit conversions
Avoid implicit conversions from signed to unsigned values, found by
UB sanitizers, by using unsigned values in the first place.

    util.c:95:15: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-09-15 10:13:30 -04:00
Topi Miettinen
92cc1b0e85 libsepol: fix typo
Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-09-13 15:41:02 +02:00
James Carter
799ef8b66e libsepol/cil: Free duplicate datums in original calling function
Duplicate declarations are allowed for type, typeattribute, and
optional statements. When an allowed duplicate declaration is found,
the duplicate datum is free'd in cil_add_decl_to_symtab() and SEPOL_OK
is returned. This works for all the rules where a duplicate declaration
is allowed, but it confuses scanning tools.

When cil_add_decl_to_symtab() finds an allowed duplicate declaration,
return SEPOL_EEXIST and free the duplicate datum in the original
calling function.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
2021-09-13 15:40:36 +02:00
Petr Lautrbach
2771dc4329 libsepol/cil: Fix detected RESOURCE_LEAK (CWE-772)
libsepol/cil/src/cil_binary.c:4823: alloc_arg: "bounds_check_type" allocates memory that is stored into "bad".
libsepol/cil/src/cil_binary.c:4840: var_assign: Assigning: "cur" = "bad".
libsepol/cil/src/cil_binary.c:4844: noescape: Resource "cur" is not freed or pointed-to in "cil_avrule_from_sepol".
libsepol/cil/src/cil_binary.c:4847: leaked_storage: Variable "cur" going out of scope leaks the storage it points to.
libsepol/cil/src/cil_binary.c:4847: leaked_storage: Variable "bad" going out of scope leaks the storage it points to.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-09-13 15:40:08 +02:00
Petr Lautrbach
38cb18e931 Update VERSIONs and Python bindings version to 3.3-rc1 for release
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-09-08 09:49:46 +02:00
James Carter
ff143e5298 libsepol/cil: Limit the number of active line marks
A line mark functions like an open parenthesis, so the number of
active line marks should be limited like the number of open
parenthesis.

This issue was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-09-08 09:45:35 +02:00
James Carter
d0b5ba03ba libsepol/cil: Add function to get number of items in a stack
Add the function, cil_stack_number_of_items(), to return the number
of items in the stack.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-08 09:45:23 +02:00
Petr Lautrbach
c304156133 libsepol: Fix detected RESOURCE_LEAKs
Fixes:
Error: RESOURCE_LEAK (CWE-772): [#def5]
libsepol/src/kernel_to_cil.c:2380: alloc_arg: "strs_init" allocates memory that is stored into "strs".
libsepol/src/kernel_to_cil.c:2386: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_cil.c:2386: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_cil.c:2386: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_cil.c:2507: leaked_storage: Variable "strs" going out of scope leaks the storage it points to.

libsepol/src/kernel_to_conf.c:2315: alloc_arg: "strs_init" allocates memory that is stored into "strs".
libsepol/src/kernel_to_conf.c:2321: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_conf.c:2321: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_conf.c:2321: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_conf.c:2385: leaked_storage: Variable "strs" going out of scope leaks the storage it points to.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-09-08 09:44:59 +02:00
James Carter
f3265d5802 libsepol/cil: Fix syntax checking in __cil_verify_syntax()
The function __cil_verify_syntax() is used to check the syntax of
CIL rules (and a few other common things like contexts and class
permissions). It does not correctly check the syntax combination
"CIL_SYN_STRING | CIL_SYN_N_LISTS, CIL_SYN_N_LISTS | CIL_SYN_END".
This should mean either a string followed by any number of lists
or any number of lists followed by the end of the rule. Instead,
while allowing the correct syntax, it allows any number of lists
followed by a string followed by any number of more lists followed
by the end of the rule and, also, any number of lists followed by a
string followed by the end of the rule.

Refactor the function to make it clearer to follow and so that once
checking begins for CIL_SYN_N_LISTS or CIL_SYN_N_STRINGS, then only
strings or lists are allowed until the end of the rule is found. In
addition, always check for CIL_SYN_END at the end.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-07 10:28:46 -04:00
James Carter
473ae1b829 libsepol/cil: Use size_t for len in __cil_verify_syntax()
Since the value passed into __cil_verify_syntax() as the len
parameter is always calculated from sizeof(syntax)/sizeof(*syntax),
use size_t for the calculated value in the calling function and for
the len parameter. In __cil_verify_syntax(), the variable i is only
compared to len, so make that size_t as well.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-07 10:28:46 -04:00
James Carter
6390a28a30 libsepol/cil: Remove redundant syntax checking
For every call to cil_fill_classperms_list(), the syntax of the
whole rule, including the class permissions, has already been
checked. There is no reason to check it again. Also, because the
class permissions appear in the middle of some rules, like
constraints, the syntax array does not end with CIL_SYN_END. This
is the only case where the syntax array does not end with CIL_SYN_END.
This prevents __cil_verify_syntax() from requiring that the syntax
array ends with CIL_SYN_END.

Remove the redundant syntax checking in cil_fill_classperms_list().

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-07 10:28:46 -04:00
James Carter
4878981229 libsepol/secilc/docs: Update the CIL documentation
Update the CIL documentation for the in-statement processing and
duplicate macro and block declarations with block inheritance.

Duplicate macro and block declarations are allowed if they occur as
the result of block inheritance. Document the fact that inherited
macros are overridden by any macros already declared in a
namespace and that declaring a block in a namespace that will
inherit a block with the same name can be used to allow in-statements
to be used on the block.

The new in-statement syntax still supports the old syntax but adds
the ability to specify whether the in-statement should be resolved
before or after block inheritance is resolved.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-07 10:28:46 -04:00
James Carter
68573634b4 libsepol/cil: Improve in-statement to allow use after inheritance
CIL's in-statement is resolved before block inheritance. This has
the advantage of allowing an in-statement to add rules to a base
block (say for a new permission) and having those rules also be
added everywhere that base block is inherited. But the disadvantage
of this behavior is that it is not possible to use an in-statement
on a block that is inherited for the simple reason that that block
does not exist when the in-statment is resolved.

Change the syntax of the in-statement to allow specifying whether
the rules should be added before or after inheritance. If neither
is specified, then the behavior remains the same. All current
in-statements will work as before.

Either the old syntax
  (in container_id
      cil_statement
      ...
  )
or the new syntax
  (in before|after container_id
      cil_statement
      ...
  )
may be used for in-statements. But only "(in after ..." will have
the new behavior. Using "(in before ..." will give the same
behavior as before.

Macro Example
;
(block b1
  (macro m1 ((type ARG1))
    (allow ARG1 self (C1 (P1a)))
  )
)
(in after b1.m1
  (allow ARG1 self (C1 (P1c)))
)
(type t1a)
(call b1.m1 (t1a))
(blockinherit b1)
(in after m1
  (allow ARG1 self (C1 (P1b)))
)
(type t1b)
(call m1 (t1b))
;
This results in the following rules:
  (allow t1a self (C1 (P1a)))
  (allow t1a self (C1 (P1c)))
  (allow t1b self (C1 (P1a)))
  (allow t1b self (C1 (P1b)))

Block Example
;
(block b2
  (block b
    (type ta)
    (allow ta self (C2 (P2a)))
  )
)
(in before b2.b
  (type tb)
  (allow tb self (C2 (P2b)))
)
(block c2
  (blockinherit b2)
  (in after b
    (type tc)
    (allow tc self (C2 (P2c)))
  )
)
;
This results in the following rules:
  (allow b2.b.ta self (C2 (P2a)))
  (allow b2.b.tb self (C2 (P2b)))
  (allow c2.b.ta self (C2 (P2a)))
  (allow c2.b.tb self (C2 (P2b)))
  (allow c2.b.tc self (C2 (P2c)))

Using in-statements on optionals also works as expected.

One additional change is that blockabstract and blockinherit rules
are not allowed when using an after in-statement. This is because
both of those are resolved before an after in-statement would be
resolved.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-07 10:28:27 -04:00
James Carter
2a6a883eed libsepol/cil: Simplify cil_tree_children_destroy()
Use a simpler recursive solution and set the head and tail pointers
of the starting node to NULL when done.

Remove the now uneeded setting of the head and tail pointers to NULL
in cil_resolve_in().

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-02 10:49:22 -04:00
James Carter
05e8f155d5 libsepol/cil: Refactor the function __cil_build_ast_node_helper()
Refactor the function __cil_build_ast_node_helper() by moving the
check for illegal statements and the large if-then-else statement
to determine which function to call to parse the policy statements
to different functions.

There is no need to keep walking the nodes of a policy statement
that has already been completely parsed. This means that the
remaining nodes of any policy statement that does not contain a list
of policy statements can be skipped. This was done inconsistently
before. The following policy statements now have all nodes after
the first one skipped: blockinherit, blockabstract, classcommon,
user, userattribute, userbounds, userprefix, type, typeattribute,
typealias, typealiasactual, typebounds, typepermissive, role,
userrole, roletype, roletransition, roleallow, roleattribute,
rolebounds, bool, tunable, typetransition, typechange, typemember,
sensitivity, sensitivityalias, senistivityaliasactual, category,
categoryalias, categoryaliasactual, and ipaddr. The only policy
statements that do contain a list of policy statements are:
block, in, tunableif, booleanif, true (conditional block), false
(conditional block), macro, optional, and src_info.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-02 10:49:18 -04:00
James Carter
3cbae1b68b libsepol/cil: Don't destroy optionals whose parent will be destroyed
If an optional that is to be disabled is the child of an optional that
is going to be disabled, then there is no reason to add that optional
to the stack of disabled optionals, because it is going to be destroyed
anyways. This means that there is no reason to maintain a stack of
disabled optionals at all.

Instead of using a stack to track disabled optionals, use a pointer
that points to the top-most optional that is to be disabled. When a
rule fails to resolve in an optional, if the disabled optional pointer
has not been set, then set it to that optional. If the pointer has
been set already, then the optional is already going to be destroyed,
so nothing else needs to be done. The resolution failure and the fact
that the optional is being disabled is reported in either case.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-02 10:49:14 -04:00
James Carter
05d1c66aaa libsepol/cil: Properly check for parameter when inserting name
File names for typetransition rules are stored in their own datums.
This allows them to be passed as a parameter, but there needs to be
a check in __cil_insert_name() so that parameter names are not
mistaken for file name strings. This check did not verify that a
matching parameter name had the flavor of CIL_NAME.

Check that the parameter flavor is CIL_NAME and that the paramter
name matches the file name to be stored in the datum.

This bug was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-02 10:48:08 -04:00
James Carter
b57535318a libsepol/cil: Reset expandtypeattribute rules when resetting AST
A list is created to store type attribute datums when resolving an
expandtypeattribute rule and that list needs to be destroyed if the
AST is reset or a memory leak will occur.

Destroy the list storing type attributes datums when resetting
expandtypeattribute rules.

This bug was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-02 10:48:02 -04:00
James Carter
4469c9796e libsepol/cil: Properly check parse tree when printing error messages
The function cil_tree_get_next_path() does not check whether the
parse tree node that stores the high-level language file path of a
src_info rule actually exists before trying to read the path. This
can result in a NULL dereference.

Check that all of the parse tree nodes of a src_info rule exist
before reading the data from them.

This bug was found by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-09-02 10:47:59 -04:00