Commit Graph

299 Commits

Author SHA1 Message Date
Petr Lautrbach
73562de8fc
Update VERSIONs to 3.4-rc1 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2022-04-06 19:53:39 +02:00
Thiébaud Weksteen
c900816e93 libsepol: Populate and use policy name
When an assertion fails, the error message refers to a generic
"policy.conf" file. When parsing a policy in checkpolicy, populate its
name using the original filename (source_filename is still build using
the #line directives within the policy).

Signed-off-by: Thiébaud Weksteen <tweek@google.com>
[Merge conflicts fixed by: James Carter <jwcart2@gmail.com>]
Signed-off-by: James Carter <jwcart2@gmail.com>
2022-02-24 10:44:46 -05:00
Christian Göttsche
4be0e2e19c checkpolicy: allow wildcard permissions in constraints
Allow all and complement permission sets in constraints, e.g.:

    constrain service ~ { status } (...);
    constrain service * (...);

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2022-02-18 14:48:57 -05:00
Christian Göttsche
01b88ac323 checkpolicy: warn on bogus IP address or netmask in nodecon statement
Warn if the netmask is not contiguous or the address has host bits set,
e.g.:

    127.0.0.0 255.255.245.0
    127.0.0.1 255.255.255.0

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-12-15 12:47:22 -05:00
Christian Göttsche
8a8275a5ac checkpolicy: ignore possible string truncation
The source code line content, saved to improve error reporting, might
get truncated, as the current Bison source buffer is 8192 bytes long and
only 254 bytes (plus NUL-terminator) are reserved.
As the saved string is only used for improving error reports and source
lines longer than 254 character are quite uncommon, simply silence the
GCC warning.

    In file included from /usr/include/string.h:519,
                     from lex.yy.c:20:
    In function ‘strncpy’,
        inlined from ‘yylex’ at policy_scan.l:63:7:
    /usr/include/x86_64-linux-gnu/bits/string_fortified.h:91:10: warning: ‘__builtin_strncpy’ output may be truncated copying 255 bytes from a string of length 8190 [-Wstringop-truncation]
       91 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
          |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-11-15 16:00:54 -05:00
Christian Göttsche
cc671d6a5e checkpolicy: use correct unsigned format specifiers
The two variables policydb_lineno and source_lineno are both of the type
unsigned long; use the appropriate format specifier.

Found by Cppcheck

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-11-15 16:00:54 -05:00
Petr Lautrbach
7f600c40bc
Update VERSIONs to 3.3 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-10-21 16:31:23 +02:00
James Carter
ce815bd11b checkpolicy: Fix potential undefined shifts
An expression of the form "1 << x" is undefined if x == 31 because
the "1" is an int and cannot be left shifted by 31.

Instead, use "UINT32_C(1) << x" which will be an unsigned int of
at least 32 bits.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-10-12 13:44:55 -04:00
Petr Lautrbach
5319c49d8a
Update VERSIONs to 3.3-rc3 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-10-06 13:28:15 +02: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
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
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
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
Christian Göttsche
1711757378
checkpolicy: mark read-only parameters in policy define 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-07-13 21:02:10 +02:00
Christian Göttsche
b306cd5b90
checkpolicy/test: mark file local functions static
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:09 +02:00
Christian Göttsche
47f4cbd357
checkpolicy: parse_util drop unused declaration
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-07-13 21:02:09 +02:00
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
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
James Carter
750cc1136d
checkpolicy: Do not automatically upgrade when using "-b" flag
When reading a binary policy, do not automatically change the version
to the max policy version supported by libsepol or, if specified, the
value given using the "-c" flag.

If the binary policy version is less than or equal to version 23
(POLICYDB_VERSION_PERMISSIVE) than do not automatically upgrade the
policy and if a policy version is specified by the "-c" flag, only set
the binary policy to the specified version if it is lower than the
current version.

If the binary policy version is greater than version 23 than it should
be set to the maximum version supported by libsepol or, if specified,
the value given by the "-c" flag.

The reason for this change is that policy versions 20
(POLICYDB_VERSION_AVTAB) to 23 have a more primitive support for type
attributes where the datums are not written out, but they exist in the
type_attr_map. This means that when the binary policy is read by
libsepol, there will be gaps in the type_val_to_struct and
p_type_val_to_name arrays and policy rules can refer to those gaps.
Certain libsepol functions like sepol_kernel_policydb_to_conf() and
sepol_kernel_policydb_to_cil() do not support this behavior and need
to be able to identify these policies. Policies before version 20 do not
support attributes at all and can be handled by all libsepol functions.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-03-15 21:52:03 +01:00
James Carter
dcd07fdcbf
libsepol/checkpolicy: Set user roles using role value instead of dominance
Roles in an optional block have two datums, one in the global block
and one in the avrule_decl where it is declared. The datum in the
global block does not have its dominace set. This is a problem because
the function set_user_role() sets the user's roles based on the global
datum's dominance ebitmap. If a user is declared with an associated role
that was declared in an optional block, then it will not have any roles
set for it because the dominance ebitmap is empty.

Example/
  # handle_unknown deny
  class CLASS1
  sid kernel
  class CLASS1 { PERM1 }
  type TYPE1;
  allow TYPE1 self:CLASS1 PERM1;
  role ROLE1;
  role ROLE1 types { TYPE1 };
  optional {
    require {
      class CLASS1 { PERM1 };
    }
    role ROLE1A;
    user USER1A roles ROLE1A;
  }
  user USER1 roles ROLE1;
  sid kernel USER1:ROLE1:TYPE1

In this example, USER1A would not have ROLE1A associated with it.

Instead of using dominance, which has been deprecated anyway, just
set the bit corresponding to the role's value in the user's roles
ebitmap in set_user_role().

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

[N.I: added spaces around "-" operator]
2021-03-15 21:50:58 +01:00
Petr Lautrbach
cf853c1a0c
Update VERSIONs to 3.2 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-03-04 16:42:59 +01:00
Petr Lautrbach
d4d1f4ba7e
Update VERSIONs to 3.2-rc3 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-24 15:49:59 +01:00
Petr Lautrbach
2c7c4a84c3
Update VERSIONs to 3.2-rc2 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-03 11:26:28 +01:00
Nicolas Iooss
b320291888 libsepol/cil: fix memory leak when a constraint expression is too deep
When __cil_validate_constrain_expr() fails,
cil_constrain_to_policydb_helper() does not destroy the constraint
expression. This leads to a memory leak reported by OSS-Fuzz with the
following CIL policy:

    (class CLASS (PERM))
    (classorder (CLASS))
    (sid SID)
    (sidorder (SID))
    (user USER)
    (role ROLE)
    (type TYPE)
    (category CAT)
    (categoryorder (CAT))
    (sensitivity SENS)
    (sensitivityorder (SENS))
    (sensitivitycategory SENS (CAT))
    (allow TYPE self (CLASS (PERM)))
    (roletype ROLE TYPE)
    (userrole USER ROLE)
    (userlevel USER (SENS))
    (userrange USER ((SENS)(SENS (CAT))))
    (sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))

    (constrain
        (CLASS (PERM))
        (or
            (eq t1 TYPE)
            (or
                (eq t1 TYPE)
                (or
                    (eq t1 TYPE)
                    (or
                        (eq t1 TYPE)
                        (or
                            (eq t1 TYPE)
                            (eq t1 TYPE)
                        )
                    )
                )
            )
        )
    )

Add constraint_expr_destroy(sepol_expr) to destroy the expression.

Moreover constraint_expr_destroy() was not freeing all items of an
expression. Code in libsepol/src and checkpolicy contained while loop to
free all the items of a constraint expression, but not the one in
libsepol/cil. As freeing only the first item of an expression is
misleading, change the semantic of constraint_expr_destroy() to iterate
the list of constraint_expr_t and to free all items.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28938
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: James Carter <jwcart2@gmail.com>
2021-02-03 09:28:39 +01:00
Petr Lautrbach
c534d4e2ce
Update VERSIONs and Python bindings version to 3.2-rc1 for release
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-01-20 12:40:14 +01:00
Nicolas Iooss
521e6a2f47 libsepol/cil: fix signed overflow caused by using (1 << 31) - 1
When compiling SELinux userspace tools with -ftrapv (this option
generates traps for signed overflow on addition, subtraction,
multiplication operations, instead of silently wrapping around),
semodule crashes when running the tests from
scripts/ci/fedora-test-runner.sh in a Fedora 32 virtual machine:

    [root@localhost selinux-testsuite]# make test
    make -C policy load
    make[1]: Entering directory '/root/selinux-testsuite/policy'
    # Test for "expand-check = 0" in /etc/selinux/semanage.conf
    # General policy build
    make[2]: Entering directory '/root/selinux-testsuite/policy/test_policy'
    Compiling targeted test_policy module
    Creating targeted test_policy.pp policy package
    rm tmp/test_policy.mod.fc
    make[2]: Leaving directory '/root/selinux-testsuite/policy/test_policy'
    # General policy load
    domain_fd_use --> off
    /usr/sbin/semodule -i test_policy/test_policy.pp test_mlsconstrain.cil test_overlay_defaultrange.cil test_add_levels.cil test_glblub.cil
    make[1]: *** [Makefile:174: load] Aborted (core dumped)

Using "coredumpctl gdb" leads to the following strack trace:

    (gdb) bt
    #0  0x00007f608fe4fa25 in raise () from /lib64/libc.so.6
    #1  0x00007f608fe38895 in abort () from /lib64/libc.so.6
    #2  0x00007f6090028aca in __addvsi3.cold () from /lib64/libsepol.so.1
    #3  0x00007f6090096f59 in __avrule_xperm_setrangebits (low=30, high=30, xperms=0x8b9eea0)
        at ../cil/src/cil_binary.c:1551
    #4  0x00007f60900970dd in __cil_permx_bitmap_to_sepol_xperms_list (xperms=0xb650a30, xperms_list=0x7ffce2653b18)
        at ../cil/src/cil_binary.c:1596
    #5  0x00007f6090097286 in __cil_avrulex_ioctl_to_policydb (k=0xb8ec200 "@\023\214\022\006", datum=0xb650a30,
        args=0x239a640) at ../cil/src/cil_binary.c:1649
    #6  0x00007f609003f1e5 in hashtab_map (h=0x41f8710, apply=0x7f60900971da <__cil_avrulex_ioctl_to_policydb>,
        args=0x239a640) at hashtab.c:234
    #7  0x00007f609009ea19 in cil_binary_create_allocated_pdb (db=0x2394f10, policydb=0x239a640)
        at ../cil/src/cil_binary.c:4969
    #8  0x00007f609009d19d in cil_binary_create (db=0x2394f10, policydb=0x7ffce2653d30) at ../cil/src/cil_binary.c:4329
    #9  0x00007f609008ec23 in cil_build_policydb_create_pdb (db=0x2394f10, sepol_db=0x7ffce2653d30)
        at ../cil/src/cil.c:631
    #10 0x00007f608fff4bf3 in semanage_direct_commit () from /lib64/libsemanage.so.1
    #11 0x00007f608fff9fae in semanage_commit () from /lib64/libsemanage.so.1
    #12 0x0000000000403e2b in main (argc=7, argv=0x7ffce2655058) at semodule.c:753

    (gdb) f 3
    #3  0x00007f6090096f59 in __avrule_xperm_setrangebits (low=30, high=30, xperms=0x8b9eea0)
        at ../cil/src/cil_binary.c:1551
    1551     xperms->perms[i] |= XPERM_SETBITS(h) - XPERM_SETBITS(low);

A signed integer overflow therefore occurs in XPERM_SETBITS(h):

    #define XPERM_SETBITS(x) ((1 << (x & 0x1f)) - 1)

This macro is expanded with h=31, so "(1 << 31) - 1" is computed:

* (1 << 31) = -0x80000000 is the lowest signed 32-bit integer value
* (1 << 31) - 1 overflows the capacity of a signed 32-bit integer and
  results in 0x7fffffff (which is unsigned)

Using unsigned integers (with "1U") fixes the crash, as
(1U << 31) = 0x80000000U has no overflowing issues.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
2020-10-15 19:25:05 +02:00
Ondrej Mosnacek
42ae834a74 libsepol,checkpolicy: optimize storage of filename transitions
In preparation to support a new policy format with a more optimal
representation of filename transition rules, this patch applies an
equivalent change from kernel commit c3a276111ea2 ("selinux: optimize
storage of filename transitions").

See the kernel commit's description [1] for the rationale behind this
representation. This change doesn't bring any measurable difference of
policy build performance (semodule -B) on Fedora.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git/commit/?id=c3a276111ea2572399281988b3129683e2a6b60b

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
2020-08-03 08:52:12 -04:00