`hashtab_search()` does take `const_hashtab_key_t` as second parameter,
which is a typedef for `const char *`.
Drop the unnecessary and const-violating cast.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Initialize variables, as they are set after goto statements, which jump
to cleanup code using them.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
`const_hashtab_key_t` is a typedef of `const char *`, so these casts are
not needed.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
../cil/src/cil_binary.c:2230:24: warning: Value stored to 'cb_node' during its initialization is never read [deadcode.DeadStores]
struct cil_tree_node *cb_node = node->cl_head;
^~~~~~~ ~~~~~~~~~~~~~
Found by clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
../cil/src/cil_write_ast.c:86:32: error: cast to smaller integer type 'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
enum cil_flavor op_flavor = (enum cil_flavor)curr->data;
^~~~~~~~~~~~~~~~~~~~~~~~~~~
../cil/src/cil_write_ast.c:130:37: error: cast to smaller integer type 'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
enum cil_flavor operand_flavor = (enum cil_flavor)curr->data;
^~~~~~~~~~~~~~~~~~~~~~~~~~~
Silence this warning by casting the pointer to an integer the cast to
enum cil_flavor.
See 32f8ed3d6b ("libsepol/cil: introduce intermediate cast to silence -Wvoid-pointer-to-enum-cast")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
In case cats_ebitmap_len() returns 0, do not allocate but quit.
Found by clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Make it more obvious which parameters are read-only and not being
modified and allow callers to pass const pointers.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Make it more obvious which parameters are read-only and not being
modified and allow callers to pass const pointers.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
conditional.c:391:4: warning: Value stored to 'i' is never read [deadcode.DeadStores]
i = 0;
^ ~
conditional.c:718:2: warning: Value stored to 'len' is never read [deadcode.DeadStores]
len = 0;
^ ~
conditional.c:772:2: warning: Value stored to 'len' is never read [deadcode.DeadStores]
len = 0;
^ ~
services.c:89:10: warning: Value stored to 'new_stack' during its initialization is never read [deadcode.DeadStores]
char **new_stack = stack;
^~~~~~~~~ ~~~~~
services.c:440:11: warning: Value stored to 'new_expr_list' during its initialization is never read [deadcode.DeadStores]
char **new_expr_list = expr_list;
^~~~~~~~~~~~~ ~~~~~~~~~
../cil/src/cil_binary.c:2230:24: warning: Value stored to 'cb_node' during its initialization is never read [deadcode.DeadStores]
struct cil_tree_node *cb_node = node->cl_head;
^~~~~~~ ~~~~~~~~~~~~~
Found by clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Follow the project style of no declaration after statement.
Found by the gcc warning -Wdeclaration-after-statement
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Follow the project style of no declaration after statement.
Found by the gcc warning -Wdeclaration-after-statement
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Unsigned integer overflow is well-defined and not undefined behavior.
But it is still useful to enable undefined behavior sanitizer checks on
unsigned arithmetic to detect possible issues on counters or variables
with similar purpose.
Use a spaceship operator like comparison instead of subtraction.
Modern compilers will generate a single comparison instruction instead
of actually perform the subtraction.
policydb.c:826:17: runtime error: unsigned integer overflow: 24 - 1699 cannot be represented in type 'unsigned int'
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The functions `role_set_get_role`, `sepol_validate_transition` and
`sepol_sidtab_remove` seem to be unused since the initial import.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Declare the functions as static or include the corresponding header
file.
assertion.c:294:5: error: no previous prototype for function 'report_assertion_failures' [-Werror,-Wmissing-prototypes]
int report_assertion_failures(sepol_handle_t *handle, policydb_t *p, avrule_t *avrule)
^
context.c:23:5: error: no previous prototype for function 'sepol_check_context' [-Werror,-Wmissing-prototypes]
int sepol_check_context(const char *context)
^
expand.c:3377:5: error: no previous prototype for function 'expand_cond_av_node' [-Werror,-Wmissing-prototypes]
int expand_cond_av_node(policydb_t * p,
^
policydb.c:638:6: error: no previous prototype for function 'role_trans_rule_destroy' [-Werror,-Wmissing-prototypes]
void role_trans_rule_destroy(role_trans_rule_t * x)
^
policydb.c:1169:5: error: no previous prototype for function 'policydb_index_decls' [-Werror,-Wmissing-prototypes]
int policydb_index_decls(sepol_handle_t * handle, policydb_t * p)
^
policydb.c:1429:6: error: no previous prototype for function 'ocontext_selinux_free' [-Werror,-Wmissing-prototypes]
void ocontext_selinux_free(ocontext_t **ocontexts)
^
policydb.c:1451:6: error: no previous prototype for function 'ocontext_xen_free' [-Werror,-Wmissing-prototypes]
void ocontext_xen_free(ocontext_t **ocontexts)
^
policydb.c:1750:5: error: no previous prototype for function 'type_set_or' [-Werror,-Wmissing-prototypes]
int type_set_or(type_set_t * dst, type_set_t * a, type_set_t * b)
^
policydb.c:2524:5: error: no previous prototype for function 'role_trans_read' [-Werror,-Wmissing-prototypes]
int role_trans_read(policydb_t *p, struct policy_file *fp)
^
policydb.c:2567:5: error: no previous prototype for function 'role_allow_read' [-Werror,-Wmissing-prototypes]
int role_allow_read(role_allow_t ** r, struct policy_file *fp)
^
policydb.c:2842:5: error: no previous prototype for function 'filename_trans_read' [-Werror,-Wmissing-prototypes]
int filename_trans_read(policydb_t *p, struct policy_file *fp)
^
services.c:1027:5: error: no previous prototype for function 'sepol_validate_transition' [-Werror,-Wmissing-prototypes]
int sepol_validate_transition(sepol_security_id_t oldsid,
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Christian Göttsche <cgzones@googlemail.com> submitted a similar patch
to quote paths when generating CIL policy from a binary policy.
Since genfscon and devicetreecon rules have paths which are allowed
to contain spaces, always quote the path when writing out these rules.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
CIL has rules that allow names to be assigned to certain objects
like MLS category sets, MLS levels, MLS ranges, IP addresses, and
class permission sets. These objects can also be named as parameters
for a macro. A call may pass in a name for one of these objects, but
it also may pass in one of the actual objects. These objects are
referred as anonymous arguments.
Add CIL policy that can be used to test whether or not anonymous
arguments are being handled properly in macros. Also test the
equivalent named arguments to help determine if the problem is with
that argument type or just with an anonymous argument of that type.
The anonymouse arguments that are tested are categoryset, level,
levelrange, ipaddr, and classpermission.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
It is possible for anonymous category sets to be in a category
expression if the expression has a macro parameter in it.
Unfortunately, anonymous category sets are not looked for when
resolving category expressions and a segfault will occur during
later processing if there was one.
As an example, consider the following portion of a policy.
(macro m1 ((categoryset cs))
(userlevel USER (s0 (cs)))
)
(call m1 ((c0 c1)))
This policy will cause a segault, because the categoryset datum
for the parameter cs is not seen as a categoryset and is treated
as a plain category.
When resolving an expression, check whether or not the datum that
is found is actually an anonymous category set associated with a
macro parameter. If it is, then resolve the category set if it
has not already been resolved and treat its categories as a sub
expression.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
A named IP address (using an ipaddr rule) could be passed as an
argument, but trying to pass an actual IP address caused an error.
As an exmample, consider the following portion of a policy.
(macro m4 ((ipaddr ip)(ipaddr nm))
(nodecon ip nm (USER ROLE TYPE ((s0) (s0))))
)
(ipaddr nm1 255.255.255.0)
(ipaddr ip1 1.2.3.4)
(call m4 (ip1 nm1)) ; This works
(call m4 (1.2.3.4 255.255.255.0)) ; This doesn't
Allow actual IP addresses to be passed as a call argument. Now the
second call works as well.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
The passing parameter "arg" of parse_module_store will be freed after
calling. A copy of parameter should be used instead of itself.
Signed-off-by: HuaxinLu <luhuaxin1@foxmail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Using mount flag `nosuid` also affects SELinux domain transitions but
this has not been documented well.
Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
When generating CIL policy from kernel or module policy quote paths,
which are allowed to contain spaces, in the statements `genfscon` and
`devicetreecon`.
Reported by LuK1337 while building policy for Android via IRC.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Improve formatting of section DESCRIPTION by adding list points.
Mention errno is set on failure.
Mention the returned context might be NULL if SELinux is not enabled.
Align setcon/_raw parameter by adding const.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
In case of a recurring call to `selinux_status_open(3)`, which
previously has been opened in fallback mode, return `1` according to its
documentation.
Fixes: c5a699046f ("libselinux: make selinux_status_open(3) reentrant")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
Anonymous levels can be passed as call arguments and they can
appear in anonymous levelranges as well.
Anonymous call arguments are resolved when they are used in a rule.
If more than one rule uses the anonymous level, then a memory leak
will occur when a new list for the category datum expression is
created without destroying the old one.
When resolving a level, check if the sensitivity datum has already
been resolved. If it has, then the categories have been as well.
Signed-off-by: James Carter <jwcart2@gmail.com>
Set the pointer to the sensitivity in levels, the pointers to the low
and high levels in levelranges, the pointer to the level in userlevels,
the pointer to the range in userranges, and the pointers to contexts
in ocontexts to NULL.
Signed-off-by: James Carter <jwcart2@gmail.com>
Anonymous class permission sets can be passed as call arguments.
Anonymous call arguments are resolved when they are used in a
rule. [This is because all the information might not be present
(like common permissions being added to a class) when the call
itself is resolved.] If there is more than one rule using an
anonymous class permission set, then a memory leak will occur
when a new list for the permission datum expression is created
without destroying the old one.
When resolving the class and permissions, check if the class has
already been resolved. If it has, then the permissions have been
as well.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
When parsing a CIL policy, the number of open parenthesis is tracked
to verify that each has a matching close parenthesis. If there are
too many open parenthesis, a stack overflow could occur during later
processing.
Exit with an error if the number of open parenthesis exceeds 4096
(which should be enough for any policy.)
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
When exiting with an error because a class or common has too many
permissions, destroy the permission nodes.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
A failed tunable resolution in a tunableif can cause an optional
to be disabled before the CIL_PASS_CALL1 phase. If this occurs, the
optional block and its subtree should be destroyed, but no reset
will be required since tunables are not allowed inside an optional
block.
Anytime there are optional blocks in the disabled_optionals list
(changed == 1), destroy the optional block and its subtree even if
in a pass before CIL_PASS_CALL1.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
The listing of the order was in the macro section, but it belongs
in the call section.
Move the listing of the order to the call section and provide a
better explanation.
Signed-off-by: James Carter <jwcart2@gmail.com>
Lorenzo Ceragioli <lorenzo.ceragioli@phd.unipi.it> noted that the
following policy:
(type a)
(block A
(macro m ((type x))
(type a)
(allow x x (file (read))))
)
(block B
(call A.m(a))
)
results in the allow rule (allow B.a B.a (file(read))). This makes
no sense because the "a" being passed as an argument has to be the
global "a" and not the "a" defined in the macro.
This behavior occurs because the call arguments are resolved AFTER
the macro body has been copied and the declaration of "a" in the
macro has been added to block B's namespace, so this is the "a"
that the call argument resolves to, rather than the one in the
global namespace.
When resolving call arguments, check if the datum found belongs to
a declaration in the call. If it does, then remove the datum from
the symbol table, re-resolve the argument, and add the datum back
into the symbol table.
Signed-off-by: James Carter <jwcart2@gmail.com>
Rename cil_resolve_call1() as cil resolve_call() and rename
cil_resolve_call2() as cil_resolve_call_args() to make it clearer
what is being done in each function.
Move code to build call arguments out of cil_resolve_call() and into
the new function called cil_build_call_args() so that the logic of
cil_resolve_call() can be seen.
Exit cil_resolve_call() immediately if the call has already been
copied.
In __cil_resolve_ast_node(), only resolve calls outside of macros.
This results in more calls to cil_copy_ast(), but slightly less
rules copied overall (since no rules are copied into a macro). This
also means that the CIL_PASS_MACRO pass is not needed and can be
eliminated.
Signed-off-by: James Carter <jwcart2@gmail.com>
Allow inserting a key without providing a node.
This will make it easier to properly resolve call arguments where
a key might need to be temporarily removed to search for a datum
that is not declared within the call. Since the node is already
in the node list, re-inserting the key without this option would
add another link to the node and cause problems.
Also, do not add the node to the datum's node list if the result
of the call to hashtab_insert() is SEPOL_EEXIST because the datum
is a duplicate and will be destroyed.
Signed-off-by: James Carter <jwcart2@gmail.com>
The CIL Reference Guide specifies how name resolution is suppose
to work within an expanded macro.
1. Items defined inside the macro
2. Items passed into the macro as arguments
3. Items defined in the same namespace of the macro
4. Items defined in the caller's namespace
5. Items defined in the global namespace
But Lorenzo Ceragioli <lorenzo.ceragioli@phd.unipi.it> found
that the first step is not done.
So the following policy:
(block A
(type a)
(macro m ()
(type a)
(allow a self (CLASS (PERM)))
)
)
(block B
(call A.m)
)
will result in:
(allow A.a self (CLASS (PERM)))
instead of the expected:
(allow B.a self (CLASS (PERM)))
Now when an expanded call is found, the macro's namespace is
checked first. If the name is found, then the name was declared
in the macro and it is declared in the expanded call, so only the
namespace of the call up to and including the global namespace
will be searched. If the name is not found in the macro's namespace
then name resolution continues with steps 2-5 above.
Signed-off-by: James Carter <jwcart2@gmail.com>
Currently `avc_init_internal()`, called by `avc_open(3)` and
`avc_init(3)`, does open the SELinux status page with fallback mode
enabled.
Quote from man:selinux_status_open(3):
In this case, this function tries to open a netlink socket using
.BR avc_netlink_open (3) and overwrite corresponding callbacks
(setenforce and policyload). Thus, we need to pay attention to the
interaction with these interfaces, when fallback mode is enabled.
Calling `selinux_status_open` internally in fallback mode is bad, cause
it overrides callbacks from client applications or the internal
fallback-callbacks get overridden by client applications.
Note that `avc_open(3)` gets called under the hood by
`selinux_check_access(3)` without checking for failure.
Also the status page is available since Linux 2.6.37, so failures of
`selinux_status_open(3)` in non-fallback mode should only be caused by
policies not allowing the client process to open/read/map
the /sys/fs/selinux/status file.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not mmap the status page again if `selinux_status_open(3)` has already
been called with success.
`selinux_status_open(3)` might be called unintentionally multiple times,
e.g. once to manually be able to call `selinux_status_getenforce(3)` and
once indirectly through `selinux_check_access(3)`
(since libselinux 3.2).
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Mention in the manpage of avc_destroy(3) that it does close the SELinux
status page, which might have been opened manually by the client
application.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
In the blockinherit section of the CIL documentation clearly state
the order in which inherited rules are resolved.
That order is:
1) The parent namespaces (if any) where the blockinherit rule is
located with the exception of the global namespace.
2) The parent namespaces of the block being inherited (but not that
block's namespace) with the exception of the global namespace.
3) The global namespace.
Signed-off-by: James Carter <jwcart2@gmail.com>
When resolving a name in a block that has been inherited. First,
a search is done in the parent namespaces (if any) of the
blockinherit rule with the exception of the global namespace. If
the name is not found, then a search is done in the namespaces of
the original block (starting with that block's namespace) with
the exception of the global namespace. Finally, if it still has
not been found, the global namespace is searched.
This does not work if a declaration is in the block being
inherited.
For example:
(block b
(typeattribute a)
(allow a self (CLASS (PERM)))
)
(blockinherit b)
This will result in a policy with the following identical allow
rules:
(allow b.a self (CLASS (PERM)))
(allow b.a self (CLASS (PERM)))
rather than the expected:
(allow b.a self (CLASS (PERM)))
(allow a self (CLASS (PERM)))
This is because when the typeattribute is copied while resolving
the inheritance, the new datum is added to the global namespace
and, since that is searched last, the typeattribute in block b is
found first.
This behavior means that no declaration that is inherited into the
global namespace will actually be used.
Instead, if the name is not found in the parent namespaces (if any)
where the blockinherit is located with the exception of the global
namespace, start the next search in the namespace of the parent of
the original block (instead of the original block itself). Now if
a declaration is inherited from the original block, the new
declaration will be used. This behavior seems to be the originally
intended behavior because there is a comment in the code that says,
"Continue search in original block's parent".
This issue was found by secilc-fuzzer. If the original block
is made to be abstract, then the type attribute declaration
in the original block is not in the policy and a segfault
occurs when creating the binary because the copied allow rule
refers to a non-existent type attribute.
Signed-off-by: James Carter <jwcart2@gmail.com>
Found by clang-tidy.
libselinux/src/label_file.c:374:4: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
else
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Found by clang-tidy.
libselinux/src/avc_sidtab.h:32:6: warning: function 'sidtab_sid_stats' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
void sidtab_sid_stats(struct sidtab *s, char *buf, int buflen) ;
^
libselinux/src/avc_sidtab.c:103:6: note: the definition seen here
void sidtab_sid_stats(struct sidtab *h, char *buf, int buflen)
^
libselinux/src/avc_sidtab.h:32:6: note: differing parameters are named here: ('s'), in definition: ('h')
void sidtab_sid_stats(struct sidtab *s, char *buf, int buflen) ;
^ ~
h
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Open the file stream with the `e` flag, so that the underlying file
descriptor gets closed on an exec in a potential sibling thread.
Also drop the flag `b`, since it is ignored on POSIX systems.
Found by clang-tidy.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
In case `realloc()` fails and returns NULL, free the passed array,
instead of just setting the size helper variables to 0.
Also free the string contents in `free_array_elts()` of the array
`con_array`, instead of just the array of pointers.
Found by cppcheck.
src/matchpathcon.c:86:4: error: Common realloc mistake: 'con_array' nulled but not freed upon failure [memleakOnRealloc]
con_array = (char **)realloc(con_array, sizeof(char*) *
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
