A require statement for a class permission adds that permission to the
class representation for the current module. In case the resulting
class would have more than the supported amount of 32 permissions
assigned the resulting binary module will fail to load at link-time
without an informative error message (since [1]).
Bail out if adding a permission would result in a class having more than
the supported amount of 32 permissions assigned.
[1]: 97af65f696
Closes: https://github.com/SELinuxProject/selinux/issues/356
Reported-by: Julie Pichon
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
With the addition of the anon_inode class in the kernel, 'self'
transition rules became useful, but haven't been implemented.
The typetransition, typemember, and typechange statements share the
relevant code, so this patch implements the self keyword in all of them
at the TE language level and adds the support to the module policydb
format. Note that changing the kernel policydb format is not necessary
at all, as type transitions are always expanded in the kernel policydb.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
When a permission for a constraint statement cannot be found also
mention the related class name.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
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>
Allow all and complement permission sets in constraints, e.g.:
constrain service ~ { status } (...);
constrain service * (...);
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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]
