In struct cil_classperms_set, the "set" field is a pointer to a
struct cil_classpermission. Normally the classpermission is created
in a classpermissionset rule with a name declared in a
classpermission rule and stored in a symbol table. Commit c49a8ea0
("libsepol/cil: cil_reset_classperms_set() should not reset
classpermission") fixed the resetting of classperms sets by setting
the "set" field to NULL rather than resetting the classpermission
that it pointed to.
But this fix mixed the special case where an anonymous classperm
set is passed as an argument to a call. In this case the
classpermission is not named and not stored in a symtab, it is
created just for the classperms set and its classperms list needs
to be reset.
Reset the classperms list if the classperms set is anonymous (which
is when the datum name is NULL).
Signed-off-by: James Carter <jwcart2@gmail.com>
Add the functions cil_write_parse_ast(), cil_write_build_ast(),
and cil_write_resolve_ast() that can be used outside of libsepol.
These functions take a FILE pointer and CIL db, do the CIL build
through the desired phase, and then call cil_write_ast() to write
the CIL AST at that point.
Signed-off-by: James Carter <jwcart2@gmail.com>
The function cil_print_tree() has existed in cil_tree.c since the
beginning of the development of CIL and secilc. Unfortunately, it
used cil_log() at log level CIL_INFO to print out the AST and
has suffered greatly from bit rot.
Move the functions to write the CIL AST to cil_write_ast.c, update
the functions, and write the AST to the FILE pointer passed in as
an argument.
The function cil_write_ast() supports writing the CIL AST at three
different phases of the compiling a CIL policy. After parsing has
been done and the parse tree has been created, after the CIL AST
has been built, and after the CIL AST has been resolved.
Signed-off-by: James Carter <jwcart2@gmail.com>
In cil_compile(), CIL_INFO is being used as the priority for
error messages. This can make it difficult to tell when the error
occurred.
Instead, use CIL_ERR as the priority for the error messages in
cil_compile().
Signed-off-by: James Carter <jwcart2@gmail.com>
Use a consistent style for the error messages when an invalid
statement is found within tunableif, in-statement, block, macro,
optional, and booleanif blocks.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since tunableifs are resolved before in-statements, do not allow
tuanble declarations in in-statements.
Since in-statements are the first flavor of statement that causes
part of the AST to be copied to another part, there is no need to
check the in-statements when resolving the AST.
Signed-off-by: James Carter <jwcart2@gmail.com>
When resolving the AST, tunable and in-statements are not considered
to be invalid in macros. This is inconsistent with the checks when
building the AST.
Add checks to make tunable and in-statments invalid in macros when
resolving the AST.
Signed-off-by: James Carter <jwcart2@gmail.com>
While there are some checks for invalid statements in an optional
block when resolving the AST, there are no checks when building the
AST.
OSS-Fuzz found the following policy which caused a null dereference
in cil_tree_get_next_path().
(blockinherit b3)
(sid SID)
(sidorder(SID))
(optional o
(ibpkeycon :(1 0)s)
(block b3
(filecon""block())
(filecon""block())))
The problem is that the blockinherit copies block b3 before
the optional block is disabled. When the optional is disabled,
block b3 is deleted along with everything else in the optional.
Later, when filecon statements with the same path are found an
error message is produced and in trying to find out where the block
was copied from, the reference to the deleted block is used. The
error handling code assumes (rightly) that if something was copied
from a block then that block should still exist.
It is clear that in-statements, blocks, and macros cannot be in an
optional, because that allows nodes to be copied from the optional
block to somewhere outside even though the optional could be disabled
later. When optionals are disabled the AST is reset and the
resolution is restarted at the point of resolving macro calls, so
anything resolved before macro calls will never be re-resolved.
This includes tunableifs, in-statements, blockinherits,
blockabstracts, and macro definitions. Tunable declarations also
cannot be in an optional block because they are needed to resolve
tunableifs. It should be fine to allow blockinherit statements in
an optional, because that is copying nodes from outside the optional
to the optional and if the optional is later disabled, everything
will be deleted anyway.
Check and quit with an error if a tunable declaration, in-statement,
block, blockabstract, or macro definition is found within an
optional when either building or resolving the AST.
Signed-off-by: James Carter <jwcart2@gmail.com>
When building the AST, typemember rules in a booleanif block will
be incorrectly called invalid. They are allowed in the kernel
policy and should be allowed in CIL.
When resolving the AST, if a neverallow rule is copied into a
booleanif block, it will not be considered an invalid rule, even
though this is not allowed in the kernel policy.
Update the booleanif checks to allow typemember rules and to not
allow neverallow rules in booleanifs. Also use the same form of
conditional for the checks when building and resolving the AST.
Signed-off-by: James Carter <jwcart2@gmail.com>
Reorder checks for invalid rules in the blocks of tunableifs,
in-statements, macros, and booleanifs when resolving the AST for
consistency.
Order the checks in the same order the blocks will be resolved in,
so tuanbleif, in-statement, macro, booleanif, and then non-block
rules.
Signed-off-by: James Carter <jwcart2@gmail.com>
When resolving the AST, block and optional stacks are used to
determine if the current rule being resolved is in a block or
an optional. There is no need to do this since the parent node
pointers can be used when exiting a block or an optional to
determine if resolution is still within a block or an optional.
When entering either a block or an optional, update the appropriate
tree node pointer. When finished with the last child of a block or
optional, set the appropriate pointer to NULL. If a parent of the
same kind is found when the parent node pointers are followed back
to the root node, then set the pointer to that tree node.
Signed-off-by: James Carter <jwcart2@gmail.com>
In order to find statements not allowed in tunableifs, in-statements,
macros, and booleanifs, there are tree node pointers that point to
each of these kinds of statements when its block is being parsed.
If the pointer is non-NULL, then the rule being parsed is in the block
of that kind of statement.
The tree node pointers were being updated at the wrong point which
prevented an invalid statement from being found if it was the first
statement in the block of a tunableif, in-statement, macro, or
booleanif.
Create a first child helper function for walking the parse tree and
in that function set the appropriate tree node pointer if the
current AST node is a tunableif, in-statement, macro, or booleanif.
This also makes the code symmetrical with the last child helper
where the tree node pointers are set to NULL.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since parse_current, finished, and extra_args can never be NULL,
remove the useless check and directly assign local variables from
extra_args.
Signed-off-by: James Carter <jwcart2@gmail.com>
Reorder checks for invalid rules in the blocks of tunableifs,
in-statements, macros, and booleanifs when building the AST for
consistency.
Order the checks in the same order the blocks will be resolved in,
so tuanbleif, in-statement, macro, booleanif, and then non-block
rules.
Signed-off-by: James Carter <jwcart2@gmail.com>
In cil_gen_node(), after the declaration is added to the symbol
table, if the parent is a macro, then a check is made to ensure
the declaration does not shadow any of the macro's parameters.
This check also needs to be done when copying the AST.
Move the check for the shadowing of macro parameters to its own
function, cil_verify_decl_does_not_shadow_macro_parameter(), and
refactor cil_gen_node() and __cil_copy_node_helper() to use the
new function.
Signed-off-by: James Carter <jwcart2@gmail.com>
The functionality of adding a declaration to a symbol table is also
needed in __cil_copy_node_helper() and not just cil_gen_node().
Create a new function called cil_add_decl_to_symtab() to add a
declaration to a symtab and refactor cil_gen_node() and
__cil_copy_node_helper() to use the new function.
By using the new function, __cil_copy_node_helper() will now allow
duplicate declarations when appropriate.
Signed-off-by: James Carter <jwcart2@gmail.com>
Change the name of cil_is_datum_multiple_decl() to
cil_allow_multiple_decls() and make it static. The new function
takes the CIL db and the flavors of the old and new datum as
arguments. Also, put all of the logic of determining if multiple
declarations are allowed into the new function. Finally, update
the call from cil_gen_node().
Signed-off-by: James Carter <jwcart2@gmail.com>
The following policy will cause a segfault:
(class CLASS (PERM))
(class C (P1 P2 P3))
(classorder (CLASS C))
(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))))
(classmap CM (PM1 PM2 PM3))
(classmapping CM PM1 (C (P1)))
(classmapping CM PM2 (C (P2)))
(classmapping CM PM3 (C (P3)))
(allow TYPE self (CM (and (all) (not PM2))))
The problem is that, while permission expressions are allowed for
normal classes, map classes are expected to only have permission
lists and no check is done to verify that only a permission list
is being used.
When the above policy is parsed, the "and" and "all" are seen as
expression operators, but when the map permissions are converted to
normal class and permissions, the permission expression is assumed
to be a list of datums and since the operators are not datums a
segfault is the result.
There is no reason to limit map classes to only using a list of
permissions and, in fact, it would be better to be able to use them
in the same way normal classes are used.
Allow permissions expressions to be used for map classes by first
evaluating the permission expression and then converting the
resulting list to normal classes and permissions.
Signed-off-by: James Carter <jwcart2@gmail.com>
When CIL parses sets or conditional expressions, any identifier that
matches an operator name will always be taken as an operator. If a
declaration has the same name as an operator, then there is the
possibility of causing either confusion or a syntax error if it is
used in an expression. The potential for problems is much greater
than any possible advantage in allowing a declaration to share the
name of a reserved word.
Create a new function, __cil_is_reserved_name() that is called when
an identifier is declared and its name is being validated. In this
function, check if the declaration has the same name as a reserved
word for an expression operator that can be used with the identifer's
flavor and exit with an error if it does.
Also, move the check for types, type aliases, and type attributes
matching the reserved word "self" to this new function.
Finally, change the name of the function __cil_verify_name() to
cil_verify_name(), since this function is neither static nor a
helper function.
Signed-off-by: James Carter <jwcart2@gmail.com>
In constraint expressions u1, u3, r1, r3, t1, and t3 are never
allowed on the right side of an expression, but there were no checks
to verify that they were not used on the right side. The result was
that the expression "(eq t1 t1)" would be silently turned into
"(eq t1 t2)" when the binary policy was created.
Verify that u1, u3, r1, r3, t1, and t3 are not used on the right
side of a constraint expression.
Signed-off-by: James Carter <jwcart2@gmail.com>
The class field of a struct cil_classperms points to the class looked
up in the symbol table, so that field should be set to NULL when
the cil_classperms is reset.
Set the class field to NULL when resetting the struct cil_classperms.
Signed-off-by: James Carter <jwcart2@gmail.com>
In struct cil_classperms_set, the set field is a pointer to a
struct cil_classpermission which is looked up in the symbol table.
Since the cil_classperms_set does not create the cil_classpermission,
it should not reset it.
Set the set field to NULL instead of resetting the classpermission
that it points to.
Signed-off-by: James Carter <jwcart2@gmail.com>
Map perms share the same struct as regular perms, but only the
map perms use the classperms field. This field is a pointer to a
list of classperms that is created and added to when resolving
classmapping rules, so the map permission doesn't own any of the
data in the list and this list should be destroyed when the AST is
reset.
When resetting a perm, destroy the classperms list without destroying
the data in the list.
Signed-off-by: James Carter <jwcart2@gmail.com>
Nicolas Iooss reports:
A few months ago, OSS-Fuzz found a crash in the CIL compiler, which
got reported as
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28648 (the title
is misleading, or is caused by another issue that conflicts with the
one I report in this message). Here is a minimized CIL policy which
reproduces the issue:
(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))))
(classpermission CLAPERM)
(optional OPT
(roletype nonexistingrole nonexistingtype)
(classpermissionset CLAPERM (CLASS (PERM)))
)
The CIL policy fuzzer (which mimics secilc built with clang Address
Sanitizer) reports:
==36541==ERROR: AddressSanitizer: heap-use-after-free on address
0x603000004f98 at pc 0x56445134c842 bp 0x7ffe2a256590 sp
0x7ffe2a256588
READ of size 8 at 0x603000004f98 thread T0
#0 0x56445134c841 in __cil_verify_classperms
/selinux/libsepol/src/../cil/src/cil_verify.c:1620:8
#1 0x56445134a43e in __cil_verify_classpermission
/selinux/libsepol/src/../cil/src/cil_verify.c:1650:9
#2 0x56445134a43e in __cil_pre_verify_helper
/selinux/libsepol/src/../cil/src/cil_verify.c:1715:8
#3 0x5644513225ac in cil_tree_walk_core
/selinux/libsepol/src/../cil/src/cil_tree.c:272:9
#4 0x564451322ab1 in cil_tree_walk
/selinux/libsepol/src/../cil/src/cil_tree.c:316:7
#5 0x5644513226af in cil_tree_walk_core
/selinux/libsepol/src/../cil/src/cil_tree.c:284:9
#6 0x564451322ab1 in cil_tree_walk
/selinux/libsepol/src/../cil/src/cil_tree.c:316:7
#7 0x5644512b88fd in cil_pre_verify
/selinux/libsepol/src/../cil/src/cil_post.c:2510:7
#8 0x5644512b88fd in cil_post_process
/selinux/libsepol/src/../cil/src/cil_post.c:2524:7
#9 0x5644511856ff in cil_compile
/selinux/libsepol/src/../cil/src/cil.c:564:7
The classperms list of a classpermission rule is created and filled
in when classpermissionset rules are processed, so it doesn't own any
part of the list and shouldn't retain any of it when it is reset.
Destroy the classperms list (without destroying the data in it) when
resetting a classpermission rule.
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
Based on patch by Nicolas Iooss, who writes:
OSS-Fuzz found a Heap-buffer-overflow in the CIL compiler when trying
to compile the following policy:
(sid SID)
(sidorder(SID))
(filecon "\" any ())
(filecon "" any ())
When cil_post_fc_fill_data() processes "\", it goes beyond the NUL
terminator of the string. Fix this by returning when '\0' is read
after a backslash.
To be consistent with the function compute_diffdata() in
refpolicy/support/fc_sort.py, also increment str_len in this case.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28484
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
In CIL, blocks, optionals, and macros share the same symbol table so
that the targets of "in" statements can be located. Because of this,
they cannot have the same name in the same namespace, but, because
they do not show up in the final policy, they can have the same name
as long as they are in different namespaces. Unfortunately, when
copying from one namespace to another, no check was being done to see
if there was a conflict.
When copying blocks, optionals, and macros, if a datum is found in
the destination namespace, then there is a conflict with a previously
declared block, optional, or macro, so exit with an error.
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Reported-by: Evgeny Vereshchagin <evvers@ya.ru>
Signed-off-by: James Carter <jwcart2@gmail.com>
If a role or user attribute with nothing associated with it is used
in a constraint expression, then the bitmap will be empty. This is
not a problem for the kernel, but does cause problems when converting
a kernel policy or module to CIL.
When creating a CIL policy from a kernel policy or module, if an
empty bitmap is encountered, use the string "NO_IDENTIFIER". An
error will occur if an attempt is made to compile the resulting
policy, but a valid policy was not being produced before anyway.
Treat types the same way even though empty bitmaps are not expected.
Signed-off-by: James Carter <jwcart2@gmail.com>
When writing CIL policy from a kernel policy or module, if there are
multiple users, roles, or types, then the list needs to be enclosed
by "(" and ")".
When writing a constraint expression, check to see if there are
multiple identifiers in the names string and enclose the list with
"(" and ")" if there are.
Signed-off-by: James Carter <jwcart2@gmail.com>
The expectation in CIL was to use user, role, or type attributes in
constraint expressions. The problem is that neither user nor role
attributes are part of the kernel binary policy, so when converting
from a kernel policy to CIL, that would require the creation of a
role or user attribute. The better solution is to just allow a list
to be used. In fact, the only thing preventing a list to be used
is a check in cil_verify_constraint_leaf_expr_syntax().
Remove the check and allow lists in constraint expressions.
The following is now allowed:
(constrain (CLASS1 (PERM1)) (eq r1 (ROLE1 ROLE2 ROLE_ATTR3)))
Signed-off-by: James Carter <jwcart2@gmail.com>
When writing a policy.conf from a kernel policy, if there are
multiple users, roles, or types, then the list needs to be enclosed
by "{" and "}".
When writing a constraint expression, check to see if there are
multiple identifiers in the names string and enclose the list
with "{" and "}" if there are.
Signed-off-by: James Carter <jwcart2@gmail.com>
If a role attribute with no roles associated with it is used in a
constraint expression, then the role bitmap will be empty. This is
not a problem for the kernel, but does cause problems when
converting a kernel policy to policy.conf.
When creating a policy.conf from a kernel policy, if an empty bitmap
is encountered, use the string "NO_IDENTIFIER". An error will occur
if an attempt is made to compile the resulting policy, but this is
better than exiting with an error without creating a policy.conf.
Signed-off-by: James Carter <jwcart2@gmail.com>
Using signed integer to represent counts can troube some gcc
optimisation passes, for example in
https://github.com/fishilico/selinux/runs/2125501324?check_suite_focus=true#step:9:107
In function ‘name_list_to_string’,
inlined from ‘constraint_expr_to_string’ at module_to_cil.c:1799:11:
module_to_cil.c:1156:8: error: argument 1 range
[18446744071562067968, 18446744073709551615] exceeds maximum
object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
1156 | str = malloc(len);
| ^~~~~~~~~~~
In file included from module_to_cil.c:39:
module_to_cil.c: In function ‘constraint_expr_to_string’:
/usr/include/stdlib.h:539:14: note: in a call to allocation
function ‘malloc’ declared here
539 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
| ^~~~~~
The wide range (from 18446744071562067968 = 0xffffffff80000000 to
18446744073709551615 = 0xffffffffffffffff) was caused by num_names being
a signed int used in "len += num_names;", even though it should always
be non-negative.
Prevent such issues from occurring by using "unsigned int" where
appropriate.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
OSS-Fuzz found a Null-dereference in __cil_insert_name when trying to
compile the following policy:
(macro MACRO ()
(classmap CLASS (PERM))
(type TYPE)
(typetransition TYPE TYPE CLASS "name" TYPE)
)
(call MACRO)
When using a macro with no argument, macro->params is NULL and
cil_list_for_each(item, macro->params) dereferenced a NULL pointer.
Fix this by checking that macro->params is not NULL before using it.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28565
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
All functions of the CIL compiler use cil_log or cil_tree_log to report
errors, but in two places which still uses printf. Replace these printf
invocation with cil_tree_log.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
printf("%i\n", node->flavor); looks very much like a statement which was
added for debugging purpose and was unintentionally left.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
cil_post_fc_fill_data() is not used outside of cil_post.c, and is not
exported in libsepol.so. Make it static, in order to ease the analysis
of static analyzers.
While at it, make its path argument "const char*" and the fields of
"struct fc_data" "unsigned int" or "size_t", in order to make the types
better match the values.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
For policy versions between 20 and 23, attributes exist in the
policy, but only in the type_attr_map. This means that there are
gaps in both the type_val_to_struct and p_type_val_to_name arrays
and policy rules can refer to those gaps which can lead to NULL
dereferences when using sepol_kernel_policydb_to_conf() and
sepol_kernel_policydb_to_cil().
This can be seen with the following policy:
class CLASS1
sid SID1
class CLASS1 { PERM1 }
attribute TYPE_ATTR1;
type TYPE1;
typeattribute TYPE1 TYPE_ATTR1;
allow TYPE_ATTR1 self : CLASS1 PERM1;
role ROLE1;
role ROLE1 types TYPE1;
user USER1 roles ROLE1;
sid SID1 USER1:ROLE1:TYPE1
Compile the policy:
checkpolicy -c 23 -o policy.bin policy.conf
Converting back to a policy.conf causes a segfault:
checkpolicy -F -b -o policy.bin.conf policy.bin
Have both sepol_kernel_policydb_to_conf() and
sepol_kernel_policydb_to_cil() exit with an error if the kernel
policy version is between 20 and 23.
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
At one point link_modules() might have needed this initial copying,
but now it serves no purpose, so remove it.
Signed-off-by: James Carter <jwcart2@gmail.com>
Types associated to role attributes in optional blocks are not
associated with the roles that have that attribute. The problem
is that role_fix_callback is called before the avrule_decls are
walked.
Example/
class CLASS1
sid kernel
class CLASS1 { PERM1 }
type TYPE1;
type TYPE1A;
allow TYPE1 self : CLASS1 PERM1;
attribute_role ROLE_ATTR1A;
role ROLE1;
role ROLE1A;
roleattribute ROLE1A ROLE_ATTR1A;
role ROLE1 types TYPE1;
optional {
require {
class CLASS1 PERM1;
}
role ROLE_ATTR1A types TYPE1A;
}
user USER1 roles ROLE1;
sid kernel USER1:ROLE1:TYPE1
In this example ROLE1A will not have TYPE1A associated to it.
Call role_fix_callback() after the avrule_decls are walked.
Signed-off-by: James Carter <jwcart2@gmail.com>
When creating the kernel binary policy, role attributes in constraint
expressions are not expanded. This causes the constraint expression
to refer to a non-existent role in the kernel policy. This can lead
to a segfault when converting the binary policy back to conf or CIL
source or when using policy tools such as seinfo.
Expand role attributes in constraint expressions when creating the
kernel binary policy.
Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
Facebook's Infer static analyzer warns about a use-after-free issue in
libsemanage:
int semanage_direct_mls_enabled(semanage_handle_t * sh)
{
sepol_policydb_t *p = NULL;
int retval;
retval = sepol_policydb_create(&p);
if (retval < 0)
goto cleanup;
/* ... */
cleanup:
sepol_policydb_free(p);
return retval;
}
When sepol_policydb_create() is called, p is allocated and
policydb_init() is called. If this second call fails, p is freed
andsepol_policydb_create() returns -1, but p still stores a pointer to
freed memory. This pointer is then freed again in the cleanup part of
semanage_direct_mls_enabled().
Fix this by setting p to NULL in sepol_policydb_create() after freeing
it.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
CIL permits not assigning a context to a SID, e.g. to an unused initial
SID, e.g. 'any_socket'.
When using the example policy from the SELinux Notebook,
https://github.com/SELinuxProject/selinux-notebook/blob/main/src/notebook-examples/cil-policy/cil-policy.cil,
secilc logs:
No context assigned to SID any_socket, omitting from policy at cil-policy.cil:166
But secil2conf segfaults when writing the policy.conf:
../cil/src/cil_policy.c:274:2: runtime error: member access within null pointer of type 'struct cil_context'
Only print the sid context statement if a context was actually assigned.
The sid declaration is still included via cil_sid_decls_to_policy().
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Nicolas Iooss reports that fuzzing /usr/libexec/hll/pp with the
American Fuzzy Lop revealed that inconsistent policy modules could be
created that caused NULL dereferences and other problems.
When reading in a binary modular or kernel policy, check values in the
policydb to verify consistency. When reading in the data for commons,
classes, roles, types, users, booleans, sensitivities, and categories
verify that their value is between 1 and the number of primary
identifiers (value-1 is used to index the sym_val_to_name array for
all of these and the val_to_struct array for classes, roles, users,
and types.) Next all references in policy rules are checked to ensure
that they refer to a valid value.
It is possible for the type and role struct and name arrays to have
gaps in them. For roles, there will be gaps in the case of a kernel
policy created from a policy with role attributes, but nothing in the
policy will refer to any of the gaps. For types, there will be gaps
for any kernel policy with a version from 20 to 23, but, unfortunately,
there will be references to the gaps. This is because, while attributes
exist in these policies, they only exist in the type_attr_map. For
policies with versions between 20 and 23, it must be assumed that all
of the gaps and any references to them are valid. To check for
references to gaps, bitmaps are created to map where the gaps are and
all values are verified to be within the proper range and not within a
gap.
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
Create the function ebitmap_highest_set_bit() which returns the position
of the highest bit set in the ebitmap.
The return value is valid only if the ebitmap is not empty. An empty
ebitmap will return 0.
Signed-off-by: James Carter <jwcart2@gmail.com>
Nicolas Iooss reports:
I am continuing to investigate OSS-Fuzz crashes and the following one
is quite complex. Here is a CIL policy which triggers a
heap-use-after-free error in the CIL compiler:
(class CLASS (PERM2))
(classorder (CLASS))
(classpermission CLSPRM)
(optional o
(mlsvalidatetrans x (domby l1 h1))
(common CLSCOMMON (PERM1))
(classcommon CLASS CLSCOMMON)
)
(classpermissionset CLSPRM (CLASS (PERM1)))
The issue is that the mlsvalidatetrans fails to resolve in pass
CIL_PASS_MISC3, which comes after the resolution of classcommon (in
pass CIL_PASS_MISC2). So:
* In pass CIL_PASS_MISC2, the optional block still exists, the
classcommon is resolved and class CLASS is linked with common
CLSCOMMON.
* In pass CIL_PASS_MISC3, the optional block is destroyed, including
the common CLSCOMMON.
* When classpermissionset is resolved, function cil_resolve_classperms
uses "common_symtab = &class->common->perms;", which has been freed.
The use-after-free issue occurs in __cil_resolve_perms (in
libsepol/cil/src/cil_resolve_ast.c):
// common_symtab was freed
rc = cil_symtab_get_datum(common_symtab, curr->data, &perm_datum);
The fundamental problem here is that when the optional block is
disabled it is immediately destroyed in the middle of the pass, so
the class has not been reset and still refers to the now destroyed
common when the classpermissionset is resolved later in the same pass.
Added a list, disabled_optionals, to struct cil_args_resolve which is
passed when resolving the tree. When optionals are disabled, they are
now added to this list and then are destroyed after the tree has been
reset between passes.
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
clang 11.0.0 reports the following warning several times (when building
with "make CC=clang" from libsepol directory, in the default
configuration of the git repository):
../cil/src/cil_binary.c:1980:8: error: cast to smaller integer type
'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
op = (enum cil_flavor)curr->data;
^~~~~~~~~~~~~~~~~~~~~~~~~~~
Silence this warning by casting the pointer to an integer the cast to
enum cil_flavor.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying
to compile the following policy:
(<src_info>)
In cil_gen_src_info(), parse_current->next is NULL even though the code
expects that both parse_current->next and parse_current->next->next
exists.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28457
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying
to compile the following policy:
(macro m((name n))) (call m(()))
When calling the macro, the name (in variable "pc") is NULL, which
triggers a NULL pointer dereference when using it as a key in
__cil_insert_name(). The stack trace is:
#0 0x7f4662655a85 in __strlen_avx2 (/usr/lib/libc.so.6+0x162a85)
#1 0x556d0b6d150c in __interceptor_strlen.part.0 (/selinux/libsepol/fuzz/fuzz-secilc+0x44850c)
#2 0x556d0ba74ed6 in symhash /selinux/libsepol/src/symtab.c:22:9
#3 0x556d0b9ef50d in hashtab_search /selinux/libsepol/src/hashtab.c:186:11
#4 0x556d0b928e1f in cil_symtab_get_datum /selinux/libsepol/src/../cil/src/cil_symtab.c:121:37
#5 0x556d0b8f28f4 in __cil_insert_name /selinux/libsepol/src/../cil/src/cil_resolve_ast.c:96:2
#6 0x556d0b908184 in cil_resolve_call1 /selinux/libsepol/src/../cil/src/cil_resolve_ast.c:2835:12
#7 0x556d0b91b404 in __cil_resolve_ast_node /selinux/libsepol/src/../cil/src/cil_resolve_ast.c
#8 0x556d0b91380f in __cil_resolve_ast_node_helper /selinux/libsepol/src/../cil/src/cil_resolve_ast.c:3773:7
#9 0x556d0b932230 in cil_tree_walk_core /selinux/libsepol/src/../cil/src/cil_tree.c:263:9
#10 0x556d0b932230 in cil_tree_walk /selinux/libsepol/src/../cil/src/cil_tree.c:307:7
#11 0x556d0b932326 in cil_tree_walk_core /selinux/libsepol/src/../cil/src/cil_tree.c:275:9
#12 0x556d0b932326 in cil_tree_walk /selinux/libsepol/src/../cil/src/cil_tree.c:307:7
#13 0x556d0b911189 in cil_resolve_ast /selinux/libsepol/src/../cil/src/cil_resolve_ast.c:3941:8
#14 0x556d0b798729 in cil_compile /selinux/libsepol/src/../cil/src/cil.c:550:7
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28544
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Nicolas Iooss reports:
OSS-Fuzz found an integer overflow when compiling the following
(empty) CIL policy:
;;*lms 2147483647 a
; (empty line)
Change hll_lineno to uint32_t which is the type of the field hll_line
in struct cil_tree_node where the line number will be stored eventually.
Read the line number into an unsigned long variable using strtoul()
instead of strtol(). On systems where ULONG_MAX > UINT32_MAX, return
an error if the value is larger than UINT32_MAX.
Also change hll_expand to uint32_t, since its value will be either
0 or 1 and there is no need for it to be signed.
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
It is good practise in C to include the header file that specifies the
prototype of functions which are defined in the source file. Otherwise,
the function prototypes which be different, which could cause unexpected
issues.
Add the include directives to do this.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In libsepol/src/mls.c, functions sepol_mls_contains and sepol_mls_check
used "sepol_policydb_t * policydb" even though
libsepol/include/sepol/context.h used "const sepol_policydb_t *
policydb".
Add const qualifiers in mls.c in order to match the header file. Detect
such mismatching error at compile time by including the header file in
mls.c.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
libsepol/src/roles.c contains functions which do not match its header
file libsepol/include/sepol/roles.h:
// In roles.c
int sepol_role_exists(sepol_handle_t * handle __attribute__ ((unused)),
sepol_policydb_t * p, const char *role, int *response)
// In roles.h
extern int sepol_role_exists(const sepol_policydb_t * policydb,
const char *role, int *response);
and:
// In roles.c
int sepol_role_list(sepol_handle_t * handle,
sepol_policydb_t * p, char ***roles, unsigned int *nroles)
// In roles.h
extern int sepol_role_list(const sepol_policydb_t * policydb,
char ***roles, unsigned int *nroles);
Instead of fixing the parameter type (using sepol_handle_t or
sepol_policydb_t but not different ones), remove these functions, as
they appear not to be used. They are not exported in libsepol.so.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
This is based on a patch by Nicolas Iooss. He writes:
When secilc compiles the following policy:
(block b1
(optional o1
(blockinherit b1)
(blockinherit x)
)
)
it disables the optional block at pass 3 (CIL_PASS_BLKIN_LINK)
because the block "x" does not exist.
__cil_resolve_ast_last_child_helper() calls
cil_tree_children_destroy() on the optional block, which destroys
the two blockinherit statements. But the (blockinherit b1) node
was referenced inside (block b1) node, in its block->bi_nodes list.
Therefore, when this list is used at pass 4 (CIL_PASS_BLKIN_COPY),
it contains a node which was freed: this triggers a use-after-free
issue
Fix this issue by removing blockinherit nodes from their lists of
nodes block->bi_nodes when they are being destroyed. As
cil_destroy_blockinherit() does not have a reference to the node
containing the blockinherit data, implement this new logic in
cil_tree_node_destroy().
This issue was found while investigating a testcase from an OSS-Fuzz
issue which seems unrelated (a Null-dereference READ in
cil_symtab_get_datum,
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29861).
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
The following CIL policy triggers a heap use-after-free in secilc
because when the blockinherit node is destroyed, the block node was
already destroyed:
(block b2a)
(blockinherit b2a)
Fix this by setting blockinherit->block to NULL when destroying block.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: James Carter <jwcart2@gmail.com>
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>
Nicolas Iooss reports:
A few weeks ago, OSS-Fuzz got configured in order to fuzz the CIL
policy compiler (cf.
https://github.com/SELinuxProject/selinux/issues/215 and
https://github.com/google/oss-fuzz/pull/4790). It reported a bunch of
simple issues, for which I will submit patches. There are also more
subtle bugs, like the one triggered by this CIL policy:
(class CLASS (PERM))
(classorder (CLASS))
(sid SID)
(sidorder (SID))
(sensitivity SENS)
(sensitivityorder (SENS))
(type TYPE)
(allow TYPE self (CLASS (PERM)))
(block b
(optional o
(sensitivitycategory SENS (C)) ; Non-existing category
triggers disabling the optional
(common COMMON (PERM1))
(classcommon CLASS COMMON)
(allow TYPE self (CLASS (PERM1)))
)
)
On my computer, secilc manages to build this policy fine, but when
clang's Address Sanitizer is enabled, running secilc leads to the
following report:
$ make -C libsepol/src CC=clang CFLAGS='-g -fsanitize=address' libsepol.a
$ clang -g -fsanitize=address secilc/secilc.c libsepol/src/libsepol.a
-o my_secilc
$ ./my_secilc -vv testcase.cil
Parsing testcase.cil
Building AST from Parse Tree
Destroying Parse Tree
Resolving AST
Failed to resolve sensitivitycategory statement at testcase.cil:12
Disabling optional 'o' at testcase.cil:11
Resetting declarations
=================================================================
==181743==ERROR: AddressSanitizer: heap-use-after-free on address
0x6070000000c0 at pc 0x55ff7e445d24 bp 0x7ffe7eecfba0 sp
0x7ffe7eecfb98
READ of size 4 at 0x6070000000c0 thread T0
#0 0x55ff7e445d23 in __class_reset_perm_values
/git/selinux-userspace/libsepol/src/../cil/src/cil_reset_ast.c:17:17
The problem is that the optional branch is destroyed when it is disabled,
so the common has already been destroyed when the reset code tries to
access the number of common permissions, so that it can change the
value of the class permissions back to their original values.
The solution is to count the number of class permissions and then
calculate the number of common permissions.
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
This field is suppose to be used to track the number of primary names in
the symtab. It was not being updated or used.
Increment the nprim field when a new datum is added to the symtab and
decrement the field when a datum is removed.
Signed-off-by: James Carter <jwcart2@gmail.com>
OSS-Fuzz found a direct memory leak in policydb_filetrans_insert()
because filenametr_destroy() does not fully destroy the list associated
with a typetransition.
More precisely, let's consider this (minimized) CIL policy:
(class CLASS (PERM))
(classorder (CLASS))
(sid SID)
(sidorder (SID))
(user USER)
(role ROLE)
(type TYPE) ; "type 1" in libsepol internal structures
(type TYPE2) ; "type 2" in libsepol internal structures
(type TYPE3) ; "type 3" in libsepol internal structures
(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))))
(typetransition TYPE2 TYPE CLASS "some_file" TYPE2)
(typetransition TYPE3 TYPE CLASS "some_file" TYPE3)
The two typetransition statements make policydb_filetrans_insert()
insert an item with key {ttype=1, tclass=1, name="some_file"} in the
hashmap p->filename_trans. This item contains a linked list of two
filename_trans_datum_t elements:
* The first one uses {otype=2, stypes=bitmap containing 2}
* The second one uses {otype=3, stypes=bitmap containing 3}
Nevertheless filenametr_destroy() (called by
hashtab_map(p->filename_trans, filenametr_destroy, NULL);) only frees
the first element. Fix this memory leak by freeing all elements.
This issue was introduced by commit 42ae834a74 ("libsepol,checkpolicy:
optimize storage of filename transitions") and was never present in the
kernel, as filenametr_destroy() was modified appropriately in commit
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c3a276111ea2572399281988b3129683e2a6b60b
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29138
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
OSS-Fuzz found a NULL pointer dereference when the CIL compiler tries to
compile a policy with an invalid integer:
$ echo '(ioportcon(2())n)' > tmp.cil
$ secilc tmp.cil
Segmentation fault (core dumped)
This is because strtol() is called with a NULL pointer, in
cil_fill_integer().
Fix this by checking that int_node->data is not NULL. While at it, use
strtoul() instead of strtol() to parse an unsigned integer.
When using "val > UINT32_MAX" with "unsigned long val;", it is expected
that some compilers emit a warning when the size of "unsigned long" is
32 bits. In theory gcc could be such a compiler (with warning
-Wtype-limits, which is included in -Wextra). Nevertheless this is
currently broken, according to
https://gcc.gnu.org/pipermail/gcc-help/2021-January/139755.html and
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89126 (this bug was
opened in January 2019).
In order to prevent this warning from appearing, introduce some
preprocessor macros around the bound check.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28456
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: James Carter <jwcart2@gmail.com>
When __cil_resolve_perms fails, it does not destroy perm_datums, which
leads to a memory leak reported by OSS-Fuzz with the following CIL
policy:
(class cl01())
(classorder(cl01))
(type at02)
(type tpr3)
(allow at02 tpr3(cl01((s))))
Calling cil_list_destroy() fixes the issue.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28466
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
OSS-Fuzz found a heap buffer overflow (out-of-bound reads) when the CIL
compiler tries to report a recursive blockinherit with an optional
block:
$ echo '(block b (optional o (blockinherit b)))' > tmp.cil
$ secilc tmp.cil
Segmentation fault (core dumped)
This is because cil_print_recursive_blockinherit() assumes that all
nodes are either CIL_BLOCK or CIL_BLOCKINHERIT. Add support for other
block kinds, using cil_node_to_string() to show them.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28462
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Function cil_add_file() copies its input into a newly-allocated buffer,
and does not modify "name". State these properties in the types of
parameters by adding "const" qualifiers.
This enables using LibFuzzer directly on cil_add_file(), without a
warning about discarding "const" qualifier:
fuzz-secilc.c: In function ‘LLVMFuzzerTestOneInput’:
fuzz-secilc.c:57:31: warning: passing argument 3 of ‘cil_add_file’
discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
57 | if (cil_add_file(db, "fuzz", data, size) != SEPOL_OK)
| ^~~~
In file included from fuzz-secilc.c:26:
/usr/include/sepol/cil/cil.h:45:57: note: expected ‘char *’ but
argument is of type ‘const uint8_t *’ {aka ‘const unsigned char *’}
45 | extern int cil_add_file(cil_db_t *db, char *name, char *data, size_t size);
| ~~~~~~^~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying
to compile the following policy:
(optional o (validatetrans x (eq t3 (a ()))))
With some logs, secilc reports:
Invalid syntax
Destroying Parse Tree
Resolving AST
Failed to resolve validatetrans statement at fuzz:1
Disabling optional 'o' at tmp.cil:1
So there is an "Invalid syntax" error, but the compilation continues.
Fix this issue by stopping the compilation when cil_fill_list() reports
an error:
Invalid syntax
Bad expression tree for constraint
Bad validatetrans declaration at tmp.cil:1
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29061
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
OSS-Fuzz found a heap use-after-free when the CIL compiler destroys its
database after failing to compile the following policy:
(validatetrans x (eq t3 (a)))
This is caused by the fact that the validatetrans AST object references
a stack variable local to __cil_fill_constraint_leaf_expr, when parsing
the list "(a)":
struct cil_list *sub_list;
cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list);
cil_list_append(*leaf_expr, CIL_LIST, &sub_list);
Drop the & sign to really add the list like it is supposed to be.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28507
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
OSS-Fuzz found a NULL pointer dereference when the CIL compiler tries to
compile a policy where a categoryalias references an unused
categoryalias:
$ echo '(categoryalias c0)(categoryalias c1)(categoryaliasactual c0 c1)' > tmp.cil
$ secil tmp.cil
Segmentation fault (core dumped)
In such a case, a1 can become NULL in cil_resolve_alias_to_actual().
Add a check to report an error when this occurs. Now the error message
is:
Alias c0 references an unused alias c1 at tmp.cil:1
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28471
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
cil_copy_expandtypeattribute prints "cil_copy_expandtypeattribute 656"
which is quite annoying. Remove the fprintf statement responsible for
this.
While at it, remove another one in cil_tree_print_node()
Fixes: https://lore.kernel.org/selinux/3c2ab876-b0b7-42eb-573d-e5b450a7125a@gmail.com/T/#u
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
It was found in https://github.com/google/oss-fuzz/pull/4790:
```
Invalid token '' at line 2 of fuzz
NEW_FUNC[1/2]: 0x67fff0 in yy_get_previous_state /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1143
NEW_FUNC[2/2]: 0x6803e0 in yy_try_NUL_trans /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1176
=================================================================
==12==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000007992 at pc 0x000000681800 bp 0x7ffccddee530 sp 0x7ffccddee528
WRITE of size 1 at 0x602000007992 thread T0
SCARINESS: 41 (1-byte-write-heap-use-after-free)
#0 0x6817ff in cil_yy_switch_to_buffer /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1315:17
#1 0x6820cc in cil_yy_scan_buffer /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1571:2
#2 0x682662 in cil_lexer_setup /src/selinux/libsepol/src/../cil/src/cil_lexer.l:73:6
#3 0x5cf2ae in cil_parser /src/selinux/libsepol/src/../cil/src/cil_parser.c:220:2
#4 0x56d5e2 in cil_add_file /src/selinux/libsepol/src/../cil/src/cil.c:514:7
#5 0x556e91 in LLVMFuzzerTestOneInput /src/secilc-fuzzer.c:434:7
#6 0x459ab1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
#7 0x45a755 in fuzzer::Fuzzer::TryDetectingAMemoryLeak(unsigned char const*, unsigned long, bool) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:675:3
#8 0x45acd9 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:747:5
#9 0x45b875 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:883:5
#10 0x4499fb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
#11 0x473a32 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#12 0x7f982296d83f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
#13 0x41e758 in _start (/out/secilc-fuzzer+0x41e758)
DEDUP_TOKEN: cil_yy_switch_to_buffer--cil_yy_scan_buffer--cil_lexer_setup
0x602000007992 is located 2 bytes inside of 4-byte region [0x602000007990,0x602000007994)
freed by thread T0 here:
#0 0x521ef2 in free /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:127:3
#1 0x56d630 in cil_add_file /src/selinux/libsepol/src/../cil/src/cil.c:526:2
#2 0x556e91 in LLVMFuzzerTestOneInput /src/secilc-fuzzer.c:434:7
#3 0x459ab1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
#4 0x458fba in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:505:3
#5 0x45acc7 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:745:19
#6 0x45b875 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:883:5
#7 0x4499fb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
#8 0x473a32 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#9 0x7f982296d83f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
DEDUP_TOKEN: free--cil_add_file--LLVMFuzzerTestOneInput
previously allocated by thread T0 here:
#0 0x52215d in malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x5cecb8 in cil_malloc /src/selinux/libsepol/src/../cil/src/cil_mem.c:39:14
#2 0x56d584 in cil_add_file /src/selinux/libsepol/src/../cil/src/cil.c:510:11
#3 0x556e91 in LLVMFuzzerTestOneInput /src/secilc-fuzzer.c:434:7
#4 0x459ab1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
#5 0x458fba in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:505:3
#6 0x45acc7 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:745:19
#7 0x45b875 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:883:5
#8 0x4499fb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
#9 0x473a32 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#10 0x7f982296d83f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
DEDUP_TOKEN: malloc--cil_malloc--cil_add_file
SUMMARY: AddressSanitizer: heap-use-after-free /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1315:17 in cil_yy_switch_to_buffer
Shadow bytes around the buggy address:
0x0c047fff8ee0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
0x0c047fff8ef0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
0x0c047fff8f00: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fd
0x0c047fff8f10: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
0x0c047fff8f20: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa
=>0x0c047fff8f30: fa fa[fd]fa fa fa fd fa fa fa fd fa fa fa fd fa
0x0c047fff8f40: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa
0x0c047fff8f50: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa
0x0c047fff8f60: fa fa fd fd fa fa fd fa fa fa fd fd fa fa fd fa
0x0c047fff8f70: fa fa 00 00 fa fa 02 fa fa fa 02 fa fa fa 00 fa
0x0c047fff8f80: fa fa 03 fa fa fa 00 fa fa fa 03 fa fa fa 00 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==12==ABORTING
```
Signed-off-by: Evgeny Vereshchagin <evvers@ya.ru>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In cil_symtab.h, the macro FLAVOR() is defined. It refers to the
flavor of the first node in the list of nodes that declare the datum.
(The flavors of every node should be the same.) While the macro was
used in many places, it was not used everywhere that it could be.
Change all the remaining places to use FLAVOR().
Signed-off-by: James Carter <jwcart2@gmail.com>
In cil_symtab.h, the macro NODE() is defined. It refers to the first
node in the list of nodes that declare that datum. (It is rare for
a datum to have more than one node in this list.) While the macro was
used in many places, it was not used everywhere that it could be.
Change all the remaining places to use NODE().
Signed-off-by: James Carter <jwcart2@gmail.com>
Block, macro, and optional names are all in stored in a block symtab. A
declarations fully-qualified name includes all of the block names from
the root node to the declaration separated by dots. Macro and optional
names are only used when trying to determine the block referred to by
an "in" block. An optional block name might be stored in a macro's
symtab, but optional blocks have no symtab and (*datum)->symtab just
refers to the symtab of the datum which would be the current symtab.
Since the assignment is not needed, remove it so the code is clearer.
Signed-off-by: James Carter <jwcart2@gmail.com>
When resolving names, the struct cil_args_resolve is passed to the
various resolve functions. The field last_resolved_name is not used.
Remove the last_resolved_name field from struct cil_args_resolve.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since cil_gen_node() is only called from declarations, the check to
determine if the node is a declaration is not needed, so remove it.
Signed-off-by: James Carter <jwcart2@gmail.com>
The function cil_tree_walk() has an argument that can be used by
the process_node helper function to tell cil_tree_walk() to skip
the node's sub-tree or the rest of the current branch. The constants
CIL_TREE_SKIP_NOTHING, CIL_TREE_SKIP_NEXT and CIL_TREE_SKIP_HEAD are
defined to be used by that argument.
Fixed two instances in the function __cil_build_ast_node_helper()
where the value 1 is used instead of the more informative
CIL_TREE_SKIP_NEXT.
Signed-off-by: James Carter <jwcart2@gmail.com>
In get_class_info(), if realloc(class_buf, new_class_buf_len) fails to
grow the memory, the function returns NULL without freeing class_buf.
This leads to a memory leak which is reported by clang's static
analyzer:
https://580-118970575-gh.circle-artifacts.com/0/output-scan-build/2020-11-11-194150-6152-1/report-42a899.html#EndPath
Fix the memory leak by calling free(class_buf).
While at it, use size_t insted of int to store the size of the buffer
which is growing.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Both tunableif and booleanif use conditional blocks (either true or
false). No ordering is imposed, so a false block can be first (or even
the only) block. Checks are made to ensure that the first and second
(if it exists) blocks are either true or false, but no checks are made
to ensure that there is only one true and/or one false block. If there
are more than one true or false block, only the first will be used and
the other will be ignored.
Create a function, cil_verify_conditional_blocks(), that gives an error
along with a message if more than one true or false block is specified
and call that function when building tunableif and booleanif blocks in
the AST.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
Previous commits removed some symbols and broke ABI, therefore we need to change
SONAME.
See the following quotes from distribution guidelines:
https://www.debian.org/doc/debian-policy/ch-sharedlibs.html#run-time-shared-libraries
Every time the shared library ABI changes in a way that may break
binaries linked against older versions of the shared library, the SONAME
of the library and the corresponding name for the binary package
containing the runtime shared library should change.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_downstream_so_name_versioning
When new versions of the library are released, you should use an ABI
comparison tool to check for ABI differences in the built shared
libraries. If it detects any incompatibilities, bump the n number by
one.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
These functions were converted to no-op by commit
c3f9492d7f ("selinux: Remove legacy local boolean and user code") and
left in libsepol/src/deprecated_functions.c to preserve API/ABI. As we
change libsepol ABI dropping duplicate symbols it's time to drop these
functions too.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Versioned duplicate symbols cause problems for LTO. These symbols were
introduced during the CIL integration several releases ago and were only
consumed by other SELinux userspace components.
Fixes: https://github.com/SELinuxProject/selinux/issues/245
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
When find_avtab_node() is called with key->specified & AVTAB_XPERMS and
xperms=NULL, xperms is being dereferenced. This is detected as a
"NULL pointer dereference issue" by static analyzers.
Even though it does not make much sense to call find_avtab_node() in a
way which triggers the NULL pointer dereference issue, static analyzers
have a hard time with calls such as:
node = find_avtab_node(handle, avtab, &avkey, cond, NULL);
... where xperms=NULL.
So, make the function report an error instead of crashing.
Here is an example of report from clang's static analyzer:
https://558-118970575-gh.circle-artifacts.com/0/output-scan-build/2020-10-02-065849-6375-1/report-d86a57.html#EndPath
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Contrary to Linux kernel, BUG_ON() does not halt the execution, in
libsepol/src/services.c. Instead it displays an error message and
continues the execution.
This means that this code does not prevent an out-of-bound write from
happening:
case CEXPR_AND:
BUG_ON(sp < 1);
sp--;
s[sp] &= s[sp + 1];
Use if(...){BUG();rc=-EINVAL;goto out;} constructions instead, to make
sure that the array access is always in-bound.
This issue has been found using clang's static analyzer:
https://558-118970575-gh.circle-artifacts.com/0/output-scan-build/2020-10-02-065849-6375-1/report-50a861.html#EndPath
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
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>
When classmaps used in a neverallow were being expanded during CIL
neverallow checking, an empty classmapping in the list of
classmappings for a classmap would cause the classmap expansion to
stop and the rest of the classmapping of the classmap to be ignored.
This would mean that not all of the classes and permissions associated
with the classmap would be used to check for a neverallow violation.
Do not end the expansion of a classmap when one classmapping is empty.
Reported-by: Jonathan Hettwer <j2468h@gmail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
CIL was not correctly determining the depth of conditional expressions
which prevented it from giving an error when the max depth was exceeded.
This allowed invalid policy binaries to be created.
Validate the conditional expression using the same logic that is used
when evaluating a conditional expression. This includes checking the
depth of the expression.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
CIL was not correctly determining the depth of constraint expressions
which prevented it from giving an error when the max depth was exceeded.
This allowed invalid policy binaries with constraint expressions exceeding
the max depth to be created.
Validate the constraint expression using the same logic that is used
when reading the binary policy. This includes checking the depth of the
the expression.
Reported-by: Jonathan Hettwer <j2468h@gmail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Implement a new, more space-efficient form of storing filename
transitions in the binary policy. The internal structures have already
been converted to this new representation; this patch just implements
reading/writing an equivalent representation from/to the binary policy.
This new format reduces the size of Fedora policy from 7.6 MB to only
3.3 MB (with policy optimization enabled in both cases). With the
unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
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>
The comparison function, portcon_data_cmp(), only made use of the
protocol to put tcp before udp, dccp, and sctp. Rules that have
the same port range, but with different protocols would be considered
equal unless one of the protocols was tcp. When generating a CIL or
conf source policy from a binary or using the "-S" option in
checkpolicy the non-tcp portcon rules with the same port range would
not be consistently sorted.
Changed portcon_data_cmp() to sort portcon rules like the CIL function
cil_post_portcon_compare().
Reported-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Improves writing of CIL category rules when converting MLS kernel
policy to CIL. No changes to functionality, but eliminate useless
checks for category aliases when using the p_cat_val_to_name array,
find the actual number of aliases before allocating memory, and
skip the category alias rules if there are no aliases.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Improves writing of CIL sensitivity rules when converting MLS kernel
policy to CIL. No changes to functionality, but eliminate useless
checks for sensitivity aliases when using the p_sens_val_to_name
array, find the actual number of aliases before allocating memory,
and skip the sensitivity alias rules if there are no aliases.
Signed-off-by: James Carter <jwcart2@gmail.com>
When converting a non-MLS kernel binary policy to CIL, write the CIL
default MLS rules (since CIL requires at least one sensitivity,
and sensitivityorder statements) on separate lines.
This improves the readability of the resulting CIL policy.
Signed-off-by: James Carter <jwcart2@gmail.com>
Type alias rules are not written out when converting a binary kernel
policy to a policy.conf. The problem is that type aliases are not in
the type_val_to_struct array and that is what is being used to find
the aliases.
Since type aliases are only in the types hashtable, walk that to
find the type aliases.
Fixed the syntax of the typalias rule which requires "alias" to come
between the type and the aliases (ex/ typealias TYPE alias ALIAS;).
Fixes: 0a08fd1e69 ("libsepol: Add ability to convert binary
policy to policy.conf file")
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Type alias rules are not written out when converting a binary kernel
policy to CIL. The problem is that type aliases are not in the
type_val_to_struct array and that is what is being used to find the
aliases.
Since type aliases are only in the types hashtable, walk that to
find the type aliases.
Fixes: 70a480bfcd ("libsepol: Add ability to convert binary
policy to CIL")
Signed-off-by: James Carter <jwcart2@gmail.com>
CIL allows a type to be redeclared when using the multiple declarations
option ("-m" or "--muliple-decls"), but make it an error for an identifier
to be declared as both a type and an attribute.
Change the error message so that it always gives the location and flavor
of both declarations. The flavors will be the same in all other cases,
but in this case they explain why there is an error even if multiple
declartions are allowed.
Fixes: Commit fafe4c212b ("libsepol: cil: Add ability to redeclare types[attributes]")
Reported-by: Topi Miettinen <toiwoton@gmail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Initialize the multiple_decls field when intializing the structure
cil_db.
Fixes: fafe4c212b ("libsepol: cil: Add ability to redeclare types[attributes]")
Reported-by: Topi Miettinen <toiwoton@gmail.com
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
As per the issue below, libsepol segfaults on loading old kernel policies
that contain duplicate filename transition rules. The segfault is due to
the fact that the val_to_name arrays have not yet been populated at this
point in the policydb_read() processing. Since this warning apparently
never worked since it was first introduced, drop it and just silently
discard the duplicate like the kernel does. I was not able to produce a
policy with such duplicates using the current policy toolchain, either
via CIL or via binary modules with manual semodule_link/expand.
Fixes: https://github.com/SELinuxProject/selinux/issues/239
Fixes: 8fdb225521 ("libsepol,checkpolicy: convert rangetrans and filenametrans to hashtabs")
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
This reverts commit 692716fc5f.
Other parts of the SELinux userspace depend on certain attributes,
such as node_type, exisiting and this change breaks those parts.
Before this patch can be reapplied, we need to identify the attributes
that must never be expanded and create a CIL module with the needed
expandtypeattribute statements (or something similar).
Signed-off-by: James Carter <jwcarter@gmail.com>
Fix issues like:
<inline asm>:1:1: error: unknown directive
.symver cil_build_policydb_pdb, cil_build_policydb@LIBSEPOL_1.0
Which was caused by the DISABLE_SYMVER define not being defined
for static, Mac or Android builds.
Acked-by: Joshua Brindle <joshua.brindle@crunchydata.com>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
Currently a constraint `t1 == t2` gets converted to the invalid cil syntax `(mlsconstrain (class_name (perm_name)) (eq t1 ))` and fails to be loaded into the kernel.
Fixes: 893851c0a1 ("policycoreutils: add a HLL compiler to convert policy packages (.pp) to CIL")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The iteration over the set ebitmap bits is not implemented very
efficiently in libsepol. It is slowing down the policy optimization
quite significantly, so convert the type_map from an array of ebitmaps
to an array of simple ordered vectors, which can be traveresed more
easily. The worse space efficiency of the vectors is less important than
the speed in this case.
After this change the duration of semodule -BN decreased from 6.4s to
5.5s on Fedora Rawhide x86_64 (and from 6.1s to 5.6s with the unconfined
module disabled).
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Only attributes can be a superset of another attribute, so we can skip
non-attributes right away.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
I copy-pasted it from a different part of the code, which had to deal
with policydb that isn't final yet. Since we only deal with the final
kernel policy here, we can skip the check for the type datum being NULL.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
With the old hidden_def and hidden_proto DSO infrastructure removed,
correctness of the map file becomes paramount, as it is what filters out
public API. Because of this, the wild cards should not be used, as it
lets some functions through that should not be made public API. Thus
remove the wild cards, and sort the list.
Additionally, verify that nothing changed in external symbols as well:
This was checked by generating an old export map (from master):
nm --defined-only -g ./src/libsepol.so | cut -d' ' -f 3-3 | grep -v '^_' > old.map
Then creating a new one for this library after this patch is applied:
nm --defined-only -g ./src/libsepol.so | cut -d' ' -f 3-3 | grep -v '^_' > new.map
And diffing them:
diff old.map new.map
Fixes: #165Fixes: #204
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
Add -fno-semantic-interposition to CFLAGS. This will restore
the DSO infrastructures protections to insure internal callers
of exported symbols call into libselinux and not something loading first
in the library list.
Clang has this enabled by default.
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
libsepol already has a linker script controlling it's exports, so this
patch has a net 0 affect, with the exception that internal callers of
external routines, which there could be 0 of, could potentially call a
non-libsepol routine depending on library load order.
NOTE A FEW SYMBOLS ARE EXPORTED THAT NORMALLY WOULDN'T BE
- sepol_context_to_sid
- sepol_ibendport_sid
- sepol_ibpkey_sid
- sepol_msg_default_handler
- sepol_node_sid
- sepol_port_sid
A subsequent map update will follow.
This list was generated by generating an old export map (from master):
nm --defined-only -g ./src/libsepol.so | cut -d' ' -f 3-3 | grep -v '^_' > old.map
Then creating a new one for this library after this patch is applied:
nm --defined-only -g ./src/libsepol.so | cut -d' ' -f 3-3 | grep -v '^_' > new.map
And diffing them:
diff old.map new.map
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: William Roberts <william.c.roberts@intel.com>
libsepol carried its own (outdated) copy of flask.h with the generated
security class and initial SID values for use by the policy
compiler and the forked copy of the security server code
leveraged by tools such as audit2why. Convert libsepol and
checkpolicy entirely to looking up class values from the policy,
remove the SECCLASS_* definitions from its flask.h header, and move
the header with its remaining initial SID definitions private to
libsepol. While we are here, fix the sepol_compute_sid() logic to
properly support features long since added to the policy and kernel,
although there are no users of it other than checkpolicy -d (debug)
and it is not exported to users of the shared library. There
are still some residual differences between the kernel logic and
libsepol.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
The value attrs_expand_size == 1 removes all empty attributes, but it
also makes sense to expand all attributes that have only one type. This
removes some redundant rules (there is sometimes the same rule for the
type and the attribute) and reduces the number of attributes that the
kernel has to go through when looking up rules.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
A parameter of a macro was only considered to be a duplicate if it
matched both the name and flavor of another parameter. While it is
true that CIL is able to differentiate between those two parameters,
there is no reason to use the same name for two macro parameters and
it is better to return an error for what is probably an error.
Remove the check of the flavors when checking for duplicate parameters.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
Type transition file names are stored in a symbol table. Before the
name is added, the symbol table is searched to see if the name had
already been inserted. If it has, then the already existing datum is
returned. If it has not, then the name is added if either the
typetransition rule does not occur in a macro or the name is not one
of the macro parameters.
Checking for a previous insertion before checking if the name is a
macro parameter can cause a macro parameter to be treated as the
actual name if a previous type transition file name is the same as
the parameter.
Now check the name to see if it a macro paramter before checking for
its existence in the symbol table.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
This reverts commit 542e878690.
After 6968ea9775 ("libsepol: make ebitmap_cardinality() of linear
complexity"), the caching only saves ~0.06 % of total semodule -BN
running time (on x86_64 without using the POPCNT instruction), so it's
no longer worth the added complexity.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Create the macro ebitmap_is_empty() to check if an ebitmap is empty.
Use ebitmap_is_empty(), instead of ebitmap_cardinality() or
ebitmap_length(), to check whether or not an ebitmap is empty.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
As ebitmap_get_bit() complexity is linear in the size of the bitmap, the
complexity of ebitmap_cardinality() is quadratic. This can be optimized
by browsing the nodes of the bitmap directly in ebitmap_cardinality().
While at it, use built-in function __builtin_popcountll() to count the
ones in the 64-bit value n->map for each bitmap node. This seems better
suited than "count++". This seems to work on gcc and clang on x86,
x86_64, ARM and ARM64 but if it causes compatibility issues with some
compilers or architectures (or with older versions of gcc or clang),
the use of __builtin_popcountll() can be replaced by a C implementation
of a popcount algorithm.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Detect when the hashtab's load factor gets too high and try to grow it
and rehash it in such case. If the reallocation fails, just keep the
hashtab at its current size, since this is not a fatal error (it will
just be slower).
This speeds up semodule -BN on Fedora from ~8.9s to ~7.2s (1.7 seconds
saved).
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
hashtab_replace() and hashtab_map_remove_on_error() aren't used
anywhere, no need to keep them around...
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
According to profiling of semodule -BN, ebitmap_cardinality() is called
quite often and contributes a lot to the total runtime. Cache its result
in the ebitmap struct to reduce this overhead. The cached value is
invalidated on most modifying operations, but ebitmap_cardinality() is
usually called once the ebitmap doesn't change any more.
After this patch, the time to do 'semodule -BN' on Fedora Rawhide has
decreased from ~10.9s to ~8.9s (2s saved).
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
[sds@tycho.nsa.gov: correct times per follow-up on list]
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
The filename_- and range_trans_table ancillary hash tables in
cil_binary.c just duplicate the final policydb content and can be simply
removed.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
The classperms associated with each map class permission and with each
classpermissionset are verified in __cil_verify_classperms() which had
multiple problems with how it did the verification.
1) Verification was short-circuited when the first normal class is found.
The second classpermissionset statement below would not have been
verified.
(classpermission cp1)
(classpermissionset cp1 (CLASS (PERM)))
(classpermissionset cp1 cp2)
2) The classperms of a map class permission and classpermissionset were
not checked for being NULL before the function recursively called itself.
This would result in a segfault if the missing map or set was referred to
before the classmap or classpermission occured. This error was reported by
Dominick Grift (dominick.grift@defensec.nl).
These rules would cause a segfault.
(classmap cm1 (mp1))
(classmapping cm1 mp1 (cm2 (mp2)))
(classmap cm2 (mp2))
But an error would be produced for these rules.
(classmap cm1 (mp1))
(classmap cm2 (mp2))
(classmapping cm2 mp2 (cm1 (mp1)))
3) The loop detection logic was incomplete and could only detect a loop
with a certain statement ordering.
These rules would cause a stack overflow.
(classmap cm1 (mp1))
(classmapping cm1 mp1 (cm2 (mp2)))
(classmap cm2 (mp2))
(classmapping cm2 mp2 (cm3 (mp3)))
(classmap cm3 (mp3))
(classmapping cm3 mp3 (cm2 (mp2)))
Rewrote __cil_verify_classperms() to fix these errors.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Add support for new SELinux policy capability genfs_seclabel_symlinks.
With this capability enabled symlinks on kernel filesystems will receive
contexts based on genfscon statements, like directories and files,
and not be restricted to the respective filesystem root sid.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Remove restrictions in libsepol and checkpolicy that required all
declared initial SIDs to be assigned a context. With this patch,
it is possible to build and load a policy that drops the sid <sidname>
<context> declarations for the unused initial SIDs. It is still
required to retain the sid <sidname> declarations (in the flask
definitions) in order to preserve the initial SID ordering/values.
The unused initial SIDs can be renamed, e.g. to add an unused_
prefix or similar, if desired, since the names used in the policy
are not stored in the kernel binary policy.
In CIL policies, the (sid ...) and (sidorder (...)) statements
must be left intact for compatibility but the (sidcontext ...)
statements for the unused initial SIDs can be omitted after this change.
With current kernels, if one removes an unused initial SID context
from policy, builds policy with this change applied and loads the
policy into the kernel, cat /sys/fs/selinux/initial_contexts/<sidname>
will show the unlabeled context. With the kernel patch to remove unused
initial SIDs, the /sys/fs/selinux/initial_contexts/<sidname>
file will not be created for unused initial SIDs in the first place.
NB If an unused initial SID was assigned a context different from
the unlabeled context in existing policy, then it is not safe to
remove that initial SID context from policy and reload policy on
the running kernel that was booted with the original policy. This
is because that kernel may have assigned that SID to various kernel
objects already and those objects will then be treated as having
the unlabeled context after the removal. In refpolicy, examples
of such initial SIDs are the "fs" SID and the "sysctl" SID. Even
though these initial SIDs are not directly used (in code) by the current
kernel, their contexts are being applied to filesystems and sysctl files by
policy and therefore the SIDs are being assigned to objects.
NB The "sysctl" SID was in use by the kernel up until
commit 8e6c96935fcc1ed3dbebc96fddfef3f2f2395afc ("security/selinux:
fix /proc/sys/ labeling) circa v2.6.39. Removing its context from
policy will cause sysctl(2) or /proc/sys accesses to end up
performing permission checks against the unlabeled context and
likely encounter denials for kernels < 2.6.39.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Commit 4459d635b8 ("libsepol: Remove cil_mem_error_handler() function
pointer") replaced cil_mem_error_handler usage with inline contents of
the default handler. However, it left over the header declaration and
two callers. Convert these as well and remove the header declaration.
This also fixes a build failure with -fno-common.
Fixes: 4459d635b8 ("libsepol: Remove cil_mem_error_handler() function pointer")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
GCC 10 comes with -fno-common enabled by default - fix the CIL_KEY_*
global variables to be defined only once in cil.c and declared in the
header file correctly with the 'extern' keyword, so that other units
including the file don't generate duplicate definitions.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
When copying an avrule with extended permissions (permx) in
cil_copy_avrule(), the check for a named permx checks the new permx
instead of the old one, so the check will always fail. This leads to a
segfault when trying to copy a named permx because there will be an
attempt to copy the nonexistent permx struct instead of the name of
the named permx.
Check whether the original is a named permx instead of the new one.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Since failing to resolve a statement in an optional block is normal,
only display messages about the statement failing to resolve and the
optional block being disabled at the highest verbosity level.
These messages are now only at log level CIL_INFO instead of CIL_WARN.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
There's a typo in commit b8213acff8 ("libsepol: add a function to optimize
kernel policy") which added new function sepol_policydb_optimize(), but there's
sepol_optimize_policy in libsepol.map.
LIBSEPOL_3.0 is used to follow the next release version libsepol-3.0
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
This improves commit b8213acf (libsepol: add a function to optimize
kernel policy) by Ondrej Mosnacek <omosnace@redhat.com> by always
removing redundant conditional rules which have an identical rule
in the unconditional policy.
Add a flag called not_cond to is_avrule_redundant(). When checking
unconditional rules against the avtab (which stores the unconditional
rules) we need to skip the actual rule that we are checking (otherwise
a rule would be determined to be redundant with itself and bad things
would happen), but when checking a conditional rule against the avtab
we do not want to skip an identical rule (which is what currently
happens), we want to remove the redundant permissions in the conditional
rule.
A couple of examples to illustrate when redundant condtional rules
are not removed.
Example 1
allow t1 t2:class1 perm1;
if (bool1) {
allow t1 t2:class1 perm1;
}
The conditional rule is clearly redundant, but without this change it
will not be removed, because of the check for an identical rule.
Example 2
typeattribute t1 a1;
allow t1 t2:class1 perm1;
allow a1 t2:class1 perm1;
if (bool1) {
allow t1 t2:class1 perm1;
}
The conditional rule is again clearly redundant, but now the order of
processing during the optimization will determine whether or not the
rule is removed. Because a1 contains only t1, a1 and t1 are considered
to be supersets of each other. If the rule with the attribute is
processed first, then it will be determined to be redundant and
removed, so the conditional rule will not be removed. But if the rule
with the type is processed first, then it will be removed and the
conditional rule will be determined to be redundant with the rule with
the attribute and removed as well.
The change reduces the size of policy a bit more than the original
optimization. Looking at the change in number of allow rules, there is
about a 10% improvement over the old optimization.
orig old new
Refpolicy 113284 82467 78053
Fedora 106410 64015 60008
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Several static analyzers (clang's one, Facebook Infer, etc.) warn about
NULL pointer dereferences after a call to CU_ASSERT_PTR_NOT_NULL_FATAL()
in the test code written using CUnit framework. This is because this
CUnit macro is too complex for them to understand that the pointer
cannot be NULL: it is translated to a call to CU_assertImplementation()
with an argument as TRUE in order to mean that the call is fatal if the
asserted condition failed (cf.
http://cunit.sourceforge.net/doxdocs/group__Framework.html).
A possible solution could consist in replacing the
CU_ASSERT_..._FATAL() calls by assert() ones, as most static analyzers
know about assert(). Nevertheless this seems to go against CUnit's API.
An alternative solution consists in overriding CU_ASSERT_..._FATAL()
macros in order to expand to assert() after a call to the matching
CU_ASSERT_...() non-fatal macro. This appears to work fine and to remove
many false-positive warnings from various static analyzers.
As this substitution should only occur when using static analyzer, put
it under #ifdef __CHECKER__, which is the macro used by sparse when
analyzing the Linux kernel.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Use codespell (https://github.com/codespell-project/codespell) in order
to find many common misspellings that are present in English texts.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In test_attr_types, the pointer decl is allowed to be NULL in the
beginning, but is dereferenced to produce a helpful message right before
a CU_ASSERT_FATAL. Make this derefence not happen if the pointer is
NULL.
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
semodule-utils/semodule_link/semodule_link.c contains:
static sepol_module_package_t *load_module(char *filename)
{
/* ... */
if (sepol_module_package_create(&p)) {
/* ... */
goto bad;
/* ... */
bad:
sepol_module_package_free(p);
When sepol_module_package_create() fails while having successfully
allocated p, it currently frees p without setting it back to NULL. This
causes a use-after-free in load_module().
Prevent this use-after-free by setting sepol_module_package_create's
argument back to NULL when an error happens.
This issue has been found using Infer static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Doing this looks wrong:
len = scope->decl_ids_len;
if (scope == NULL) {
/* ... */
Move the dereferencing of scope after the NULL check.
This issue has been found using Infer static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When strs_stack_init(&stack) fails to allocate memory and stack is still
NULL, it should not be dereferenced with strs_stack_pop(stack).
This issue has been found using Infer static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
As reported by Nicolas Iooss (nicolas.iooss@m4x.org), static analyzers
have problems understanding that the default memory error handler does
not return since it is called through the cil_mem_error_handler()
function pointer. This results in a number of false positive warnings
about null pointer dereferencing.
Since the ability to set the cil_mem_error_handler() is only through
the function cil_set_mem_error_handler() which is never used and whose
definition is not in any header file, remove that function, remove the
use of cil_mem_error_handler() and directly in-line the contents of
the default handler, cil_default_mem_error_handler().
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
This patch is loosely based on a patch by Yuli Khodorkovskiy
<yuli@crunchydata.com> from June 13th, 2019.
Since any permission used in the policy should be defined, CIL
should return an error if it cannot resolve a permission used
in a policy. This was the original behavior of CIL.
The behavior was changed over three commits from July to November
2016 (See commits 46e157b47, da51020d6, and 2eefb20d8). The change
was motivated by Fedora trying to remove permissions from its
policy that were never upstreamed (ex/ process ptrace_child and
capability2 compromise_kernel). Local or third party modules
compiled with those permissions would break policy updates.
After three years it seems unlikely that we need to worry about
those local and third party modules and it is time for CIL to
give an error like it should.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Policy developers can set a default_range default to glblub and
computed contexts will be the intersection of the ranges of the
source and target contexts. This can be used by MLS userspace
object managers to find the range of clearances that two contexts
have in common. An example usage is computing a transition between
the network context and the context of a user logging into an MLS
application.
For example, one can add a default with
this cil:
(defaultrange db_table glblub)
or in te (base module only):
default_range db_table glblub;
and then test using the compute_create utility:
$ ./compute_create system_u:system_r:kernel_t:s0:c1,c2,c5-s0:c1.c20 system_u:system_r:kernel_t:s0:c0.c20-s0:c0.c36 db_table
system_u:object_r:kernel_t:s0:c1,c2,c5-s0:c1.c20
Some example range transitions are:
User Permitted Range | Network Device Label | Computed Label
---------------------|----------------------|----------------
s0-s1:c0.c12 | s0 | s0
s0-s1:c0.c12 | s0-s1:c0.c1023 | s0-s1:c0.c12
s0-s4:c0.c512 | s1-s1:c0.c1023 | s1-s1:c0.c512
s0-s15:c0,c2 | s4-s6:c0.c128 | s4-s6:c0,c2
s0-s4 | s2-s6 | s2-s4
s0-s4 | s5-s8 | INVALID
s5-s8 | s0-s4 | INVALID
Signed-off-by: Joshua Brindle <joshua.brindle@crunchydata.com>
