When write_binary_policy() fails to open the binary policy, it calls
sepol_handle_destroy(f.handle) but structure f has not been initialized
at this point. Use variable handle instead.
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
f.handle is never set in read_binary_policy() so there is no need to
call sepol_handle_destroy() on it. Moreover clang's static analyzer
warns about an uninitialized argument value in the first call.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
va_copy()'s manpage [1] states:
Each invocation of va_copy() must be matched by a corresponding
invocation of va_end() in the same function.
create_str_helper() is using va_copy() without va_end(). Add the missing
call.
[1] https://linux.die.net/man/3/va_copy
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
sepol_ppfile_to_module_package() does not use its variable "FILE *f =
NULL;" but to fclose() it. This variable has been unneeded since the
introduction of function ppfile_to_module_package() in commit
893851c0a1 ("policycoreutils: add a HLL compiler to convert policy
packages (.pp) to CIL").
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
clang's static analyzer reports that ebitmap_to_names() can call
malloc(0) when the bitmap is empty. If malloc() returns NULL, this
triggers a misleading "Out of memory" error.
Work around this by treating empty bitmaps as appropriate.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
clang's static analyzer warns about dead assignments to local variables.
In module_to_cil.c, there are some which are quite straightforward to
review. Remove them.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When list_prepend() returns an error, it always means it failed to
allocate some memory and does not hold any reference to its argument
data. This argument needs to be freed by the caller in order to prevent
a memory leak.
While reviewing list_prepend() callers, I spend quite some time
understanding why typealiases_gather_map() does not need to strdup(key)
or free(key) when calling list_prepend(..., key) even though "key" comes
from pdb->p_types.table: because typealias_list_destroy() does not free
the inserted items. Add a comment to make this clearer in the code.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In cil_tree_print_expr(), "rc < 0" is equivalent to "rc != 0" but
clang's static analyzer does not know about this. Help it.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Fix the following build warning:
policydb.c: In function ‘get_symtab_destroy_func’:
policydb.c:1581:9: error: cast between incompatible function types from ‘int (*)(char *, void *, void *)’ to ‘void (*)(char *, void *, void *)’ [-Werror=cast-function-type]
return (hashtab_destroy_func_t) destroy_f[sym_num];
^
It turns out that this function and type are long unused in libsepol
and are not exported APIs for the shared library, so just remove them.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
When load_users() parses an invalid line with an empty level context
(ie. nothing between "level" and "range" keywords), it allocates memory
with malloc(0) and uses it. The behavior of malloc() in this case is
an unspecified behavior: it might return NULL, which would lead to a
segmentation fault.
Fix this issue by reporting the invalid entry instead. While at it,
ensure that the character before "range" is a space, and change the
logic slightly in order to avoid using "--p; ... p++;".
This issue is reported by clang's static analyzer with the following
message:
genusers.c:222:11: warning: Use of zero-allocated memory
*r++ = *s;
^
genusers.c:225:7: warning: Use of zero-allocated memory
*r = 0;
^
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In cond_expr_to_cil(), when stack_init() fails to allocate a stack, the
function calls stack_pop() with stack = NULL. Then stack_pop()
dereferences the pointer ("if (stack->pos == -1) {"), which is NULL.
Fix this by moving the stack cleaning loop in a "if (stack != NULL)"
block.
This issue is reported by clang's static analyzer with the following
message:
module_to_cil.c:463:6: warning: Access to field 'pos' results in a
dereference of a null pointer (loaded from variable 'stack')
if (stack->pos == -1) {
^~~~~~~~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Improve the processing of netifcon, genfscon, ibpkeycon, ibendportcon,
portcon, nodecon, fsuse, filecon, iomemcon, ioportcon, pcidevicecon,
and devicetreecon rules.
If the multiple-decls option is not used then report errors if duplicate
context rules are found. If it is used then remove duplicate context rules
and report errors when two rules are identical except for the context.
This also changes the ordering of portcon and filecon rules. The protocol
of portcon rules will be compared if the port numbers are the same and the
path strings of filecon rules will be compared if the number of meta
characters, the stem length, string length and file types are the same.
Based on an initial patch by Pierre-Hugues Husson (phh@phh.me)
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
This commit resolves conflicts in values of expandattribute statements
in policy language and expandtypeattribute in CIL.
For example, these statements resolve to false in policy language:
expandattribute hal_audio true;
expandattribute hal_audio false;
Similarly, in CIL these also resolve to false.
(expandtypeattribute (hal_audio) true)
(expandtypeattribute (hal_audio) false)
A warning will be issued on this conflict.
Motivation
When Android combines multiple .cil files from system.img and vendor.img
it's possible to have conflicting expandattribute statements.
This change deals with this scenario by resolving the value of the
corresponding expandtypeattribute to false. The rationale behind this
override is that true is used for reduce run-time lookups, while
false is used for tests which must pass.
Signed-off-by: Tri Vo <trong@android.com>
Acked-by: Jeff Vander Stoep <jeffv@google.com>
Acked-by: William Roberts <william.c.roberts@intel.com>
Acked-by: James Carter <jwcart2@tycho.nsa.gov>
Nicolas Iooss reports:
In sepol_ibendport_key_create(), if sepol_ibendport_alloc_ibdev_name()
fails to allocate tmp_key->ibdev_name, sepol_ibendport_key_free() is
called to free the memory associated with tmp_key, which results in
free() being called on uninitialized tmp_key->ibdev_name.
This issue is reported by clang's static analyzer with the following
message:
ibendport_record.c:115:2: warning: 1st function call argument is an
uninitialized value
free(key->ibdev_name);
^~~~~~~~~~~~~~~~~~~~~
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Export the sepol_polcap_getnum/name() functions to users of
the shared library. This will enable SETools to stop depending
on the static library.
Note that we may want to move polcaps.h up one level since
the convention is that headers directly under include/sepol are
shared library APIs while headers under include/sepol/policydb
are limited to static users. However, this will unnecessarily
break the build for existing static users so it is deferred.
Suggested-by: Chris PeBenito <pebenito@ieee.org>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
cil_tree_print_expr() calls cil_expr_to_string() in order to compute a
string expression into expr_str. If this function fails, expr_str is
left unitialized but its value is dereferenced with:
cil_log(CIL_INFO, "%s)", expr_str);
Prevent such an issue by checking cil_expr_to_string()'s return value
before using expr_str.
This issue has been found with clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
This patch solves the following issues:
- The pkg-config files generates odd paths when using DESTDIR without PREFIX
- DESTDIR is needed during compile time to compute library and header paths which it should not.
- Installing with both DESTDIR and PREFIX set gives us odd paths
- Make usage of DESTDIR and PREFIX more standard
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Daniel Cashman <dcashman@android.com> discovered the following:
When using cil_db multiple_decls, the different cil_attribute nodes
all point to the same underlying cil_attribute struct. This leads
to problems, though, when modifying the used value in the struct.
__cil_post_db_attr() changes the value of the field to based on
the output of cil_typeattribute_used(), for use later in
cil_typeattribute_to_policydb and cil_typeattribute_to_bitmap, but
due to the multiple declarations, cil_typeattribute_used() could be
called again by a second node. In this second call, the value used
is the modifed value of CIL_TRUE or CIL_FALSE, not the flags actually
needed. This could result in the field being reset again, to an
incorrect CIL_FALSE value.
Add the field "keep" to struct cil_typeattributeset, set its value
using cil_typeattribute_used(), and use it when determining whether
the attribute is to be kept or if it should be expanded.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
When reading policy, ibendport device names are allocated in
ocontext_read_selinux() but they are not freed when calling
sepol_policydb_free();
Fix this by freeing them in ocontext_selinux_free().
Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
When sepol_bool_query() returns NULL response, variable name is not
freed. Fix this by calling free() before returning.
Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
cil_gen_node() has been using its argument "db" since commit
fafe4c212b ("libsepol: cil: Add ability to redeclare
types[attributes]"). Drop attribute "unused" on this argument.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
cil_defaults_to_policy() defines its third argument as non-const "char
*kind" even though it is called with literal strings. This makes gcc
report the following warning when compiling with -Wwrite-strings:
../cil/src/cil_policy.c: In function ‘cil_gen_policy’:
../cil/src/cil_policy.c:1931:60: error: passing argument 3 of
‘cil_defaults_to_policy’ discards ‘const’ qualifier from pointer
target type [-Werror=discarded-qualifiers]
cil_defaults_to_policy(out, lists[CIL_LIST_DEFAULT_USER],
"default_user");
^~~~~~~~~~~~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Since commit 67b410e80f ("libsepol/cil: Keep attributes used by
generated attributes in neverallow rules") gcc reports the following
warning when building libsepol:
../cil/src/cil_post.c: In function
‘__cil_post_db_neverallow_attr_helper’:
../cil/src/cil_post.c:1322:17: error: unused variable ‘db’
[-Werror=unused-variable]
struct cil_db *db = extra_args;
^~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When compiling libsepol with clang and some warning flags, the compiler
complains about the way IPv6 addresses are initialized:
kernel_to_cil.c:2795:35: error: suggest braces around initialization
of subobject [-Werror,-Wmissing-braces]
struct in6_addr subnet_prefix = {0};
^
{}
When replacing the initializer as suggested, gcc 4.8.4 complains:
kernel_to_cil.c: In function ‘write_selinux_ibpkey_rules_to_cil’:
kernel_to_cil.c:2795:9: error: missing initializer for field
‘__in6_u’ of ‘struct in6_addr’ [-Werror=missing-field-initializers]
struct in6_addr subnet_prefix = {};
^
Thankfully netinet/in.h provides a macro to initialize struct in6_addr
variables:
#define IN6ADDR_ANY_INIT { { { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 } } }
Both clang and gcc no longer report warnings when using this macro.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In order to reduce policy size, CIL removes attributes that are not used
by a policy rule in the generated binary policy. However, CIL keeps
attributes used by neverallow rules (which are checked at compile time
and not in the binary policy) even if the attribute is not used anywhere
else in the policy. This behavior is useful to Google who pulls neverallow
rules out of the original policy.conf for compatibility testing, but
converts the policy.conf to CIL and uses the CIL compiler to generate
policy. Without this behavior, the generated binary policy might not have
an attribute referred to by one of the neverallow rules used for testing.
The one exception to this behavior is for attributes generated in
module_to_cil (these have an "_typeattr_" in the middle of their name).
Since these attributes are only created because CIL does not allow a
type expression in an AV rule, they are removed if they only appear in
a neverallow rule (which is the case for most of them) or if the
option to expand generated attributes (-G or --expand-generated) is
specified for secilc when compiling the policy.
Removing generated attributes causes a problem, however, if the type
expression that the generated attribute is replacing uses an attribute
that is removed. In this case, the original neverallow rule will refer
to an attribute that does not exist in the generated binary policy.
Now any non-generated attribute used in a typeattributeset rule for a
generated attribute which is used in a neverallow rule will be treated
like it was used in a neverallow rule.
This does not change the behavior of an expandtypeattribute rule for
the attribute. That rule, if it exists, will take precedence.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Modify cil_gen_node() to check to see if the cil_db supports multiple
declarations, and if so, to check whether or not the
repeated symbol is eligible to share the existing, already-stored datum. The
only types considered so far are CIL_TYPE and CIL_TYPEATTRIBUTE, both of
which intall empty datums during AST building, so they automatically return
true.
Test: Build policy with multilpe type and attribute declarations, and
without. Policies are binary-identical.
Signed-off-by: Dan Cashman <dcashman@android.com>
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
In cil_strpool_destroy(), cil_strpool_tab is freed but it is not reset to NULL.
When cil_strpool_init() is called again it assumes that cil_strpool_tab was
already initialized. Other functions then work with invalid data.
Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
Define the nnp_nosuid_transition policy capability used to enable
SELinux domain transitions under NNP or nosuid if the
nnp_transition permission or nosuid_transition permission is
allowed between the old and new contexts. When this capability is not
enabled, such transitions remain limited to bounded transitions as they
were prior to the introduction of this capability.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
A build toolchain may override CPPFLAGS on the command line of "make".
Doing so currently breaks libsepol/tests compilation, as it requires
"-I../include/ -I$(CHECKPOLICY)" to be provided in gcc's arguments.
This completes commit 15f2740733 ("Makefiles: override *FLAGS and
*LIBS").
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
There were several places in the makefiles where LDLIBS or CFLAGS were
supposed to include options to build. They were missing the override
keyword so would be skipped if these vars were set on the make cmdline.
Add the override directive to fix this.
Signed-off-by: Jason Zaman <jason@perfinion.com>
While most header files already use the common pattern of `extern "C"`
declarations to enable compiling in a C++ project, some header files in
libsepol instead use the macros `__BEGIN_DECLS` and `__END_DECLS`. These
macros are defined in the "sys/cdefs.h" header file, which provides
some non-standard extensions for glibc.
Convert usage of these declarations with the standard `extern "C"`
pattern. This improves compatibility with other libc implementations,
e.g. musl libc.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
The typebounds rules should end with a ";".
The netifcon and nodecon rules should not end with a ";".
The default rules are missing a "_". They should be "default_user",
"default_role" and "default_type".
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Commit 9e6840e refactored neverallow checking. In the process a bug
was introduced that causes enabled conditional rules to be skipped.
The bug is that the avtab key is checked by comparing the specified
field of the key to the value AVTAB_ALLOWED. Since enabled conditional
rules have an additional bit set as well, these rules are not
considered to match.
The fix is to use a bitwise AND (&) to only check the desired bit.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
