Commit Graph

3061 Commits

Author SHA1 Message Date
Christian Göttsche
4e3d0990c6
checkpolicy: drop redundant cast to the same type
Found by clang-tidy.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:09 +02:00
Christian Göttsche
5218bf4b26
checkpolicy: avoid potential use of uninitialized variable
checkpolicy.c: In function ‘main’:
    checkpolicy.c:1000:25: error: ‘tsid’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     1000 |                         printf("if_sid %d default_msg_sid %d\n", ssid, tsid);
          |                         ^

    checkpolicy.c: In function ‘main’:
    checkpolicy.c:971:25: error: ‘tsid’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
      971 |                         printf("fs_sid %d default_file_sid %d\n", ssid, tsid);
          |                         ^

Found by GCC 11 with LTO enabled.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:09 +02:00
Christian Göttsche
5a10f05f53
checkpolicy: check before potential NULL dereference
policy_define.c: In function ‘define_te_avtab_extended_perms’:
    policy_define.c:1946:17: error: potential null pointer dereference [-Werror=null-dereference]
     1946 |         r->omit = omit;
          |                 ^

In the case of `r` being NULL, avrule_read_ioctls() would return
with its parameter `rangehead` being a pointer to NULL, which is
considered a failure in its caller `avrule_ioctl_ranges`.
So it is not necessary to alter the return value.

Found by GCC 11 with LTO enabled.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:08 +02:00
Christian Göttsche
7723180fa0
checkpolicy: remove dead assignments
The variable `cladatum` is otherwise always assigned before used, so
these two assignments without a follow up usages are not needed.

Found by clang-analyzer.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:08 +02:00
Christian Göttsche
79e7724930
checkpolicy: 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>
2021-07-13 21:02:07 +02:00
Christian Göttsche
babc3d5351
checkpolicy: use correct format specifier for unsigned
test/dispol.c:288:4: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
       snprintf(buf, sizeof(buf), "unknown (%d)", i);
       ^
    test/dismod.c:830:4: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
       snprintf(buf, sizeof(buf), "unknown (%d)", i);
       ^

Found by Cppcheck.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:06 +02:00
Christian Göttsche
db674bf218
checkpolicy: drop dead condition
The variable `id` is guaranteed to be non-NULL due to the preceding
while condition.

    policy_define.c:1171:7: style: Condition '!id' is always false [knownConditionTrueFalse]
      if (!id) {
          ^
    policy_define.c:1170:13: note: Assuming that condition 'id=queue_remove(id_queue)' is not redundant
     while ((id = queue_remove(id_queue))) {
                ^
    policy_define.c:1171:7: note: Condition '!id' is always false
      if (!id) {
          ^

Found by Cppcheck.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:05 +02:00
Christian Göttsche
7cdb2a8fd2
checkpolicy: simplify assignment
checkpolicy.c:504:20: style: The statement 'if (policyvers!=n) policyvers=n' is logically equivalent to 'policyvers=n'. [duplicateConditionalAssign]
    if (policyvers != n)
                   ^
checkpolicy.c:505:17: note: Assignment 'policyvers=n'
     policyvers = n;
                ^
checkpolicy.c:504:20: note: Condition 'policyvers!=n' is redundant
    if (policyvers != n)
                   ^

Found by Cppcheck

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:05 +02:00
Christian Göttsche
02678b9d40
checkpolicy: drop -pipe compile option
The compiler option -pipe does not affect the generated code; it affects
whether the compiler uses temporary files or pipes. As the benefit might
vary from system to system usually its up to the packager or build
framework to set it.
Also these are the only places where the flag is used.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:04 +02:00
Christian Göttsche
40e2f98519
checkpolicy: pass CFLAGS at link stage
Pass CFLAGS when invoking CC at link time, it might contain optimization
or sanitizer flags required for linking.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:04 +02:00
Christian Göttsche
07d6f1cea5
libsepol: assure string NUL-termination of ibdev_name
Clang complains:

    ibendport_record.c: In function ‘sepol_ibendport_get_ibdev_name’:
    ibendport_record.c:169:2: error: ‘strncpy’ specified bound 64 equals destination size [-Werror=stringop-truncation]
      169 |  strncpy(tmp_ibdev_name, ibendport->ibdev_name, IB_DEVICE_NAME_MAX);
          |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    ibendport_record.c: In function ‘sepol_ibendport_set_ibdev_name’:
    ibendport_record.c:189:2: error: ‘strncpy’ specified bound 64 equals destination size [-Werror=stringop-truncation]
      189 |  strncpy(tmp, ibdev_name, IB_DEVICE_NAME_MAX);
          |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

strncpy(3) does not NUL-terminate the destination if the source is of
the same length or longer then the specified size.
The source of these copies are retrieved from
sepol_ibendport_alloc_ibdev_name(), which allocates a fixed amount of
IB_DEVICE_NAME_MAX bytes.
Reduce the size to copy by 1 of all memory regions allocated by
sepol_ibendport_alloc_ibdev_name().

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:01:11 +02:00
Christian Göttsche
e1491388d5
libsepol: avoid implicit conversions
Avoid implicit conversions from signed to unsigned values, found by
UB sanitizers, by using unsigned values in the first place.

    expand.c:1644:18: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)

    expand.c:2892:24: runtime error: implicit conversion from type 'int' of value -2 (32-bit, signed) to type 'unsigned int' changed the value to 4294967294 (32-bit, unsigned)

    policy_define.c:2344:4: runtime error: implicit conversion from type 'int' of value -1048577 (32-bit, signed) to type 'unsigned int' changed the value to 4293918719 (32-bit, unsigned)

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:01:07 +02:00
Christian Göttsche
09405ba91c
libsepol: ignore UBSAN false-positives
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.

Annotate functions, in which unsigned overflows are expected to happen,
with the respective Clang function attribute[1].
GCC does not support sanitizing unsigned integer arithmetic[2].

    avtab.c:76:2: runtime error: unsigned integer overflow: 6 * 3432918353 cannot be represented in type 'unsigned int'
    policydb.c:795:42: runtime error: unsigned integer overflow: 8160943042179512010 * 11 cannot be represented in type 'unsigned long'
    symtab.c:25:12: runtime error: left shift of 1766601759 by 4 places cannot be represented in type 'unsigned int'

[1]: https://clang.llvm.org/docs/AttributeReference.html#no-sanitize
[2]: https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:01:03 +02:00
Christian Göttsche
44d56761be
libsepol: avoid unsigned integer overflow
Unsigned integer overflow is well-defined and not undefined behavior.
It is commonly used for hashing or pseudo random number generation.
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 or missed overflow checks on user input.

Use a spaceship operator like comparison instead of subtraction.

    policydb.c:851:24: runtime error: unsigned integer overflow: 801 - 929 cannot be represented in type 'unsigned int'

Follow-up of: 1537ea8412 ("libsepol: avoid unsigned integer overflow")

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:00:33 +02:00
James Carter
9af9169241 libsepol/cil: Improve checking for bad inheritance patterns
commits 37863b0b14 (libsepol/cil:
Improve degenerate inheritance check) and
74d00a8dec (libsepol/cil: Detect
degenerate inheritance and exit with an error) attempted to detect
and exit with an error when compiling policies that have degenerate
inheritances. These policies result in the exponential growth of memory
usage while copying the blocks that are inherited.

There were two problems with the previous attempts to detect this
bad inheritance problem. The first is that the quick check using
cil_possible_degenerate_inheritance() did not detect all patterns
of degenerate inheritance. The second problem is that the detection
of inheritance loops during the CIL_PASS_BLKIN_LINK pass did not
detect all inheritance loops which made it possible for the full
degenerate inheritance checking done with
cil_check_for_degenerate_inheritance() to have a stack overflow
when encountering the inheritance loops. Both the degenerate and
loop inheritance checks need to be done at the same time and done
after the CIL_PASS_BLKIN_LINK pass. Otherwise, if loops are being
detected first, then a degenerate policy can cause the consumption
of all system memory and if degenerate policy is being detected
first, then an inheritance loop can cause a stack overflow.

With the new approach, the quick check is eliminated and the full
check is always done after the CIL_PASS_BLKIN_LINK pass. Because
of this the "inheritance_check" field in struct cil_resolve_args
is not needed and removed and the functions
cil_print_recursive_blockinherit(), cil_check_recursive_blockinherit(),
and cil_possible_degenerate_inheritance() have been deleted. The
function cil_count_potential() is renamed cil_check_inheritances()
and has checks for both degenerate inheritance and inheritance loops.
The inheritance checking is improved and uses an approach similar
to commit c28525a26f (libsepol/cil:
Properly check for loops in sets).

As has been the case with these degenerate inheritance patches,
these issues were discovered by the secilc-fuzzer.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-07-07 12:29:15 -04:00
Nicolas Iooss
e293718f0e mcstrans: silence -Wextra-semi-stmt warning
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:

      mcstransd.c:72:35: error: empty expression statement has no effect;
      remove unnecessary ';' to silence this warning
      [-Werror,-Wextra-semi-stmt]
              log_debug("%s\n", "cleanup_exit");
                                               ^

Replace the empty log_debug substitution with a do { ... } while (0)
construction to silence this warning.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-07-06 11:08:11 -04:00
Nicolas Iooss
e45bc87094 policycoreutils: silence -Wextra-semi-stmt warning
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:

      secon.c:686:3: error: empty expression statement has no effect;
      remove unnecessary ';' to silence this warning
      [-Werror,-Wextra-semi-stmt]
              };
               ^

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-07-06 11:08:11 -04:00
Nicolas Iooss
ed7e3348d1 checkpolicy: silence -Wextra-semi-stmt warning
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:

      checkpolicy.c:740:33: error: empty expression statement has no
      effect; remove unnecessary ';' to silence this warning
      [-Werror,-Wextra-semi-stmt]
                      FGETS(ans, sizeof(ans), stdin);
                                                    ^

Introduce "do { } while (0)" blocks to silence such warnings.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-07-06 11:08:11 -04:00
Nicolas Iooss
e1c6df329c libsemanage: silence -Wextra-semi-stmt warning
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:

      genhomedircon.c:742:67: error: empty expression statement has no
      effect; remove unnecessary ';' to silence this warning
      [-Werror,-Wextra-semi-stmt]
              const semanage_seuser_t **u2 = (const semanage_seuser_t **) arg2;;
                                                                               ^

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-07-06 11:08:11 -04:00
Nicolas Iooss
40543dceed libselinux: silence -Wextra-semi-stmt warning
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:

      sha1.c:90:21: error: empty expression statement has no effect;
      remove unnecessary ';' to silence this warning
      [-Werror,-Wextra-semi-stmt]
          R0(a,b,c,d,e, 0); R0(e,a,b,c,d, 1); R0(d,e,a,b,c, 2); R0(c,d,e,a,b, 3);
                          ^
      In file included from selinux_restorecon.c:39:
      ./label_file.h:458:15: error: empty expression statement has no
      effect; remove unnecessary ';' to silence this warning
      [-Werror,-Wextra-semi-stmt]
                                  lineno);
                                        ^

Introduce "do { } while (0)" blocks to silence such warnings.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-07-06 11:08:11 -04:00
Nicolas Iooss
9d85aa60d1 libsepol: silence -Wextra-semi-stmt warning
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:

  ../cil/src/cil_binary.c:4293:22: error: empty expression statement
  has no effect; remove unnecessary ';' to silence this warning
  [-Werror,-Wextra-semi-stmt]
          mix(k->target_class);
                              ^
  ../cil/src/cil_binary.c:4294:21: error: empty expression statement
  has no effect; remove unnecessary ';' to silence this warning
  [-Werror,-Wextra-semi-stmt]
          mix(k->target_type);
                             ^
  ../cil/src/cil_binary.c:4295:21: error: empty expression statement
  has no effect; remove unnecessary ';' to silence this warning
  [-Werror,-Wextra-semi-stmt]
          mix(k->source_type);
                             ^
  ../cil/src/cil_binary.c:4296:19: error: empty expression statement
  has no effect; remove unnecessary ';' to silence this warning
  [-Werror,-Wextra-semi-stmt]
          mix(k->specified);
                           ^

Use a do { ... } while (0) construction to silence this warning.

Moreover the same warning appears when using two semicolons to end a
statement. Remove such occurrences, like what was already done in commit
811185648a ("libsepol: drop repeated semicolons").

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-07-06 11:08:11 -04:00
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
Fabrice Fontaine
4859b73813
libselinux/utils/getseuser.c: fix build with gcc 4.8
Fix the following build failure with gcc 4.8 which is raised since
version 3.2 and
156dd0de5c

getseuser.c:53:2: error: 'for' loop initial declarations are only allowed in C99 mode
  for (int i = 0; i < n; i++)
  ^

Fixes:
 - http://autobuild.buildroot.org/results/37eb0952a763256fbf6ef3c668f6c95fbdf2dd35

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
2021-07-03 16:00:38 +02:00
James Carter
532a4cc336
libsepol/cil: Add support for using qualified names to secil2conf
Provide the option "-Q" or "--qualified-names" to indicate that the
policy is using qualified names.

Using qualified names means that declaration names can have "dots"
in them, but blocks, blockinherits, blockabstracts, and in-statements
are not allowed in the policy.

The libsepol function cil_set_qualified_names() is called with the
desired value for the CIL db's "qualified_names" field.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-07-03 16:00:30 +02:00
James Carter
74c06d763f
libsepol/cil: Add support for using qualified names to secil2tree
Provide the option "-Q" or "--qualified-names" to indicate that the
policy is using qualified names.

Using qualified names means that declaration names can have "dots"
in them, but blocks, blockinherits, blockabstracts, and in-statements
are not allowed in the policy.

The libsepol function cil_set_qualified_names() is called with the
desired value for the CIL db's "qualified_names" field.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-07-03 16:00:30 +02:00
James Carter
f7b8b5055f
secilc: Add support for using qualified names to secilc
Provide the option "-Q" or "--qualified-names" to indicate that the
policy is using qualified names.

Using qualified names means that declaration names can have "dots"
in them, but blocks, blockinherits, blockabstracts, and in-statements
are not allowed in the policy.

The libsepol function cil_set_qualified_names() is called with the
desired value for the CIL db's "qualified_names" field.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-07-03 16:00:30 +02: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