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>
In mls_semantic_range_expand(), when a call to
mls_semantic_level_expand() fails, the function destroys the semantic
level instead of the expanded one. This leads to a use-after-free which
is reported by gcc's Address Sanitizer:
libsepol.mls_semantic_level_expand: mls_semantic_level_expand: invalid sensitivity level found 128/0.
libsepol.sepol_module_package_read: invalid module in module package (at section 0)
Failed to read policy package
=================================================================
==24456==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200000ee58 at pc 0x7fe6c4fb96b4 bp 0x7fffa5ea6b70 sp 0x7fffa5ea6b60
READ of size 8 at 0x60200000ee58 thread T0
#0 0x7fe6c4fb96b3 in mls_semantic_level_destroy /usr/src/selinux/libsepol/src/mls.c:755
#1 0x7fe6c4fb9b88 in mls_semantic_range_destroy /usr/src/selinux/libsepol/src/mls.c:802
#2 0x7fe6c500e8ab in user_datum_destroy /usr/src/selinux/libsepol/src/policydb.c:535
#3 0x7fe6c500e980 in user_destroy /usr/src/selinux/libsepol/src/policydb.c:1390
#4 0x7fe6c4f36c48 in hashtab_map /usr/src/selinux/libsepol/src/hashtab.c:235
#5 0x7fe6c50152da in symtabs_destroy /usr/src/selinux/libsepol/src/policydb.c:1595
#6 0x7fe6c5015433 in policydb_destroy /usr/src/selinux/libsepol/src/policydb.c:1503
#7 0x7fe6c5040e0d in sepol_policydb_free /usr/src/selinux/libsepol/src/policydb_public.c:82
#8 0x7fe6c4fbc503 in sepol_module_package_free /usr/src/selinux/libsepol/src/module.c:143
#9 0x7fe6c4fefefb in sepol_ppfile_to_module_package /usr/src/selinux/libsepol/src/module_to_cil.c:4293
#10 0x401e51 in main /usr/src/selinux/policycoreutils/hll/pp/pp.c:124
#11 0x7fe6c4add510 in __libc_start_main (/usr/lib/libc.so.6+0x20510)
#12 0x402589 in _start (/usr/src/selinux/DESTDIR/usr/libexec/selinux/hll/pp+0x402589)
0x60200000ee58 is located 8 bytes inside of 16-byte region [0x60200000ee50,0x60200000ee60)
freed by thread T0 here:
#0 0x7fe6c5537ae0 in __interceptor_free /build/gcc-multilib/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:45
#1 0x7fe6c4fb969b in mls_semantic_level_destroy /usr/src/selinux/libsepol/src/mls.c:757
#2 0x7fe6c4f02a57 in mls_semantic_range_expand /usr/src/selinux/libsepol/src/expand.c:948
#3 0x7fe6c5007a98 in policydb_user_cache /usr/src/selinux/libsepol/src/policydb.c:939
#4 0x7fe6c4f36c48 in hashtab_map /usr/src/selinux/libsepol/src/hashtab.c:235
#5 0x7fe6c5013859 in policydb_index_others /usr/src/selinux/libsepol/src/policydb.c:1286
#6 0x7fe6c5020b65 in policydb_read /usr/src/selinux/libsepol/src/policydb.c:4342
#7 0x7fe6c4fc0cdb in sepol_module_package_read /usr/src/selinux/libsepol/src/module.c:618
#8 0x7fe6c4ff008d in sepol_ppfile_to_module_package /usr/src/selinux/libsepol/src/module_to_cil.c:4276
#9 0x401e51 in main /usr/src/selinux/policycoreutils/hll/pp/pp.c:124
#10 0x7fe6c4add510 in __libc_start_main (/usr/lib/libc.so.6+0x20510)
previously allocated by thread T0 here:
#0 0x7fe6c5537e40 in __interceptor_malloc /build/gcc-multilib/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:62
#1 0x7fe6c5004efc in mls_read_semantic_level_helper /usr/src/selinux/libsepol/src/policydb.c:1976
#2 0x7fe6c500f596 in mls_read_semantic_range_helper /usr/src/selinux/libsepol/src/policydb.c:2010
#3 0x7fe6c500f596 in user_read /usr/src/selinux/libsepol/src/policydb.c:3258
#4 0x7fe6c502055b in policydb_read /usr/src/selinux/libsepol/src/policydb.c:4286
#5 0x7fe6c4fc0cdb in sepol_module_package_read /usr/src/selinux/libsepol/src/module.c:618
#6 0x7fe6c4ff008d in sepol_ppfile_to_module_package /usr/src/selinux/libsepol/src/module_to_cil.c:4276
#7 0x401e51 in main /usr/src/selinux/policycoreutils/hll/pp/pp.c:124
#8 0x7fe6c4add510 in __libc_start_main (/usr/lib/libc.so.6+0x20510)
SUMMARY: AddressSanitizer: heap-use-after-free /usr/src/selinux/libsepol/src/mls.c:755 in mls_semantic_level_destroy
Shadow bytes around the buggy address:
0x0c047fff9d70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff9d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff9d90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff9da0: fa fa fa fa fa fa fa fa fa fa 01 fa fa fa 01 fa
0x0c047fff9db0: fa fa 01 fa fa fa 01 fa fa fa 01 fa fa fa 01 fa
=>0x0c047fff9dc0: fa fa 00 00 fa fa 00 00 fa fa fd[fd]fa fa fd fd
0x0c047fff9dd0: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fd
0x0c047fff9de0: fa fa 04 fa fa fa 00 01 fa fa fd fd fa fa fd fd
0x0c047fff9df0: fa fa fd fd fa fa fd fd fa fa 00 00 fa fa fd fd
0x0c047fff9e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff9e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 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
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
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
==24456==ABORTING
This issue has been found while fuzzing hll/pp with the American Fuzzy
Lop.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Commit 1089665e31 (Add attribute
expansion options) adds an expandattribute rule to the policy.conf
language which sets a type_datum flag. Currently the flag is used
only when writing out CIL policy from a policy.conf.
Make use of the flag when expanding policy to expand policy rules
and remove all type associations for an attribute that has
TYPE_FLAGS_EXPAND_ATTR_TRUE set. (The attribute will remain in the
policy, but have no types associated with it.)
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
Nicolas Iooss <nicolas.iooss@m4x.org> discovered with clang's static
analyzer that cil_reset_ibpkeycon() was checking that ibpkeycon->context
was NULL and then passing the NULL value to cil_reset_context() which
expected a non-NULL argument.
Instead, cil_reset_ibpkeycon() should check if ibpkeycon->context_str
is NULL. If it is non-NULL then the context field points to a named
context that was created elsewhere and it will be reset there, but if
the context_str field is NULL, then the context is not named and needs
to be reset.
Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
