In case of malloc error, ctx1, or ctx2 may be pointing to uninitialized
space and context_destroy should not be used on it.
Fixes:
Error: UNINIT (CWE-457):
libsepol-3.7/src/mls.c:673:2: alloc_fn: Calling "malloc" which returns uninitialized memory.
libsepol-3.7/src/mls.c:673:2: assign: Assigning: "ctx1" = "malloc(64UL)", which points to uninitialized data.
libsepol-3.7/src/mls.c:699:2: uninit_use_in_call: Using uninitialized value "ctx1->range.level[0].cat.node" when calling "context_destroy".
\# 697| ERR(handle, "could not check if mls context %s contains %s",
\# 698| mls1, mls2);
\# 699|-> context_destroy(ctx1);
\# 700| context_destroy(ctx2);
\# 701| free(ctx1);
Error: UNINIT (CWE-457):
libsepol-3.7/src/mls.c:674:2: alloc_fn: Calling "malloc" which returns uninitialized memory.
libsepol-3.7/src/mls.c:674:2: assign: Assigning: "ctx2" = "malloc(64UL)", which points to uninitialized data.
libsepol-3.7/src/mls.c:700:2: uninit_use_in_call: Using uninitialized value "ctx2->range.level[0].cat.node" when calling "context_destroy".
\# 698| mls1, mls2);
\# 699| context_destroy(ctx1);
\# 700|-> context_destroy(ctx2);
\# 701| free(ctx1);
\# 702| free(ctx2);
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
This capability can be enabled to change the kernel's behaviour and use
the extended permissions for netlink messages.
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Add support for AVTAB_XPERMS_NLMSG as extended permissions for netlink
sockets. The behaviour is similar to the existing
AVTAB_XPERMS_IOCTLFUNCTION.
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
The function cil_gen_alias() is used to declare type, sensitivity,
and category aliases and the function cil_gen_aliasactual() is used
to assign an alias to the actual declared name.
Commit e55621c03 ("libsepol/cil: Add notself and other support to CIL")
added "notself" and "other" as reserved words. Previously, a check
was made in cil_gen_aliasactual() to ensure that the "self" reserved
word was not used. With the notself patch this function was upgraded
to call cil_verify_name() to verify that the other reserved words
were not used as well. This change prevents the use of dotted names
to refer to alias or actual names that are declared in blocks.
The check for a reserved word being used is not needed because that
check will be done for both the alias and the actual name when they
are declared.
Remove the call to cil_verify_name() and allow dotted names in
aliasactual rules.
Reported-by: Dominick Grift <dominick.grift@defensec.nl>
Signed-off-by: James Carter <jwcart2@gmail.com>
Make sure sym_index is within the bounds of symtab array before using it
to index the array.
Fixes:
Error: OVERRUN (CWE-119):
libsepol-3.6/cil/src/cil_resolve_ast.c:3157: assignment: Assigning: "sym_index" = "CIL_SYM_UNKNOWN".
libsepol-3.6/cil/src/cil_resolve_ast.c:3189: overrun-call: Overrunning callee's array of size 19 by passing argument "sym_index" (which evaluates to 20) in call to "cil_resolve_name".
\# 3187| switch (curr->flavor) {
\# 3188| case CIL_STRING:
\# 3189|-> rc = cil_resolve_name(parent, curr->data, sym_index, db, &res_datum);
\# 3190| if (rc != SEPOL_OK) {
\# 3191| goto exit;
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
Validate that the permission maps in the scope index refer to a valid
class datum. Otherwise since commit 52e5c306 ("libsepol: move unchanged
data out of loop") this can lead to a NULL dereference in the class
existence check during linking.
Reported-by: oss-fuzz (issue 69655)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Commit 1c91bc84 ("libsepol: reject self flag in type rules in old
policies") actually rejects all type rules in conditionals in modular
policies prior to version 21 (MOD_POLICYDB_VERSION_SELF_TYPETRANS).
The problem is because of fall-through in a switch statement when
the avrule flags are 0. Instead, break rather than fall-through when
avrule flags are 0.
Reviewed-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <lautrbach@redhat.com>
Ensure the attribute-to-type maps contain no invalid entries, required
for generating typeattributeset statements when converting to CIL.
Reported-by: oss-fuzz (issue 69283)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Check the class is defined once, and not for every permission via
is_perm_enabled(). Also pass the class datum to avoid an unnecessary
name lookup.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Perform the lookup whether the class is in the current scope once, and
not for every permission.
This also ensures the class is checked to be in the current scope if
there are no permissions attached.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
When the comparison function returns 0, avoid a repeated call to it.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
If writing a policy fails due to a limitation by the requested policy
version include a prefix if the version refers to a module policy.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Validate the type-to-associated-attributes maps also for policies prior
to version 20.
To ensure only valid entries in these maps, skip the degenerate case for
gaps during construction.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Kernel policy versions 20 to 23 store attributes only in type_attr_map
and reference gaps in the type arrays. Thus they are exempted from gaps
checks.
Only exempt kernel policies, not base and module ones.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The flag RULE_SELF in type rules is only supported in modular policies
since version 21 (MOD_POLICYDB_VERSION_SELF_TYPETRANS).
Reported-by: oss-fuzz (issue 68731)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
libsepol-3.6/cil/src/cil_binary.c:902: alloc_fn: Storage is returned from allocation function "cil_malloc".
libsepol-3.6/cil/src/cil_binary.c:902: var_assign: Assigning: "mls_level" = storage returned from "cil_malloc(24UL)".
libsepol-3.6/cil/src/cil_binary.c:903: noescape: Resource "mls_level" is not freed or pointed-to in "mls_level_init".
libsepol-3.6/cil/src/cil_binary.c:905: noescape: Resource "mls_level" is not freed or pointed-to in "mls_level_cpy".
libsepol-3.6/cil/src/cil_binary.c:919: leaked_storage: Variable "mls_level" going out of scope leaks the storage it points to.
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
Validate the symbol tables for permissions of security classes and
common classes:
* check their value is valid
* check their values are unique
* check permission values of classes do not reuse values from
inherited permissions
This simplifies validating permissions of access vectors a lot, since it
is now only a binary and against the valid permission mask of the class.
Use UINT32_MAX instead of 0 as the special value for validating
constraints signaling a validate-trans rule, since classes with no
permissions are permitted, but they must not have a normal constraint
attached.
Reported-by: oss-fuzz (issue 67893)
Improves: 8c64e5bb6f ("libsepol: validate access vector permissions")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
If a policy version cannot be found include the policy target, and a
module prefix for non kernel policies in the message.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The function pointer arrays are never changed, declare them const.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Since commit c205b924e2 ("libsepol: Fix buffer overflow when using
sepol_av_to_string()") writing an access vector with no valid permission
results in an error instead of an empty string being written.
Validate that at least one permission of an access vector is valid.
There might be invalid bits set, e.g. by previous versions of
checkpolicy setting all bits for the wildcard (*) permission.
Reported-by: oss-fuzz (issue 67730)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Commit e81c466 "Fix class permission verification in CIL", added a
check for the use of "all" in a permission expression for a class
that had no permissions. Unfortunately, that change did not take
into account a class that had common permissions, so a class that
has no permmissions of its own, but inherits permissions from a
common, will fail the verification check.
If the class inherits from a common, then add those permissions to
the permmission list when verifying the permission expression.
Example/
(common co1 (cop1))
(class cl1 ())
(classcommon cl1 co1)
(classorder (CLASS cl1))
(classpermission cp1)
(classpermissionset cp1 (cl1 (all)))
(classmap cm1 (cmp1))
(classmapping cm1 cmp1 (cl1 (all)))
Previously, both the classpermissionset and the classmapping rules
would fail verification, but now they pass as expected.
Patch originally from Ben Cressey <bcressey@amazon.com>, I have
expanded the explanation.
Reported-by: Ben Cressey <bcressey@amazon.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
In libselinux there is an availability check for strlcpy() and
in both libselinux and libsepol there are availability checks for
reallocarray() in the src Makfiles. CFLAGS and LDFLAGS are needed
for cross-compiling, but, unfortunately, the default CFLAGS cause
all of these availability checks to fail to compile because of
compilationerrors (rather than just the function not being available).
Add CFLAGS and LDFLAGS to the availibility checks, update the checks
so that a compilation error will only happen if the function being
checked for is not available, and make checks for the same function
the same in both libselinux and libsepol.
Suggested-by: Jordan Williams <jordan@jwillikers.com>
Suggested-by: Winfried Dobbe <winfried_mb2@xmsnet.nl>
Signed-off-by: James Carter <jwcart2@gmail.com>
If MLS support is enabled check the policy version supports MLS.
Reported-by: oss-fuzz (issue #67322)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The function sepol_av_to_string() normally returns a list of
permissions with a space at the beginning, but it will return '\0'
if there are no permissions. Unfortunately, functions in
kernel_to_cil, kernel_to_conf, and module_to_cil assume there is a
space at the beginning and skip the space by using "perms+1".
In kernel_to_cil, kernel_to_conf, and module_to_cil, check for the
permission string being '\0' and return an error if it is.
Reported-by: oss-fuzz (issue 67276)
Signed-off-by: James Carter <jwcart2@gmail.com>
In checkpolicy, a sensitivity that has one or more aliases will
temporarily share the mls_level_t structure with its aliases until
a level statement is processed for the sensitivity (or one of the
aliases) and the aliases are updated to have their own mls_level_t
structure. If the policydb is destroyed while they are sharing the
mls_level_t structure, then a double free of the shared mls_level_t
will occur. This does not currently occur only because checkpolicy
does very little clean-up before exiting.
The "defined" field of the level_datum_t is set after a level
statement is processed for a sensitivity and its aliases. This means
that we know an alias has its own mls_level_t if the "defined" field
is set. The double free can be avoided by not destroying the
mls_leve_t structure for an alias unless the "defined" field is set.
Since the "defined" field is only set to false while the mls_level_t
structure is being shared, it would be clearer to rename the field
as "notdefined". It would only be set during the time the sensitivity
and its aliases are sharing the mls_level_t structure. Outside of
checkpolicy, the "notdefined" field will always be set to 0.
Also, do more validation of the level_datum_t when validating the
policydb.
Signed-off-by: James Carter <jwcart2@gmail.com>
Ensure comparison functions used by qsort(3) fulfill transitivity, since
otherwise the resulting array might not be sorted correctly or worse[1]
in case of integer overflows.
[1]: https://www.qualys.com/2024/01/30/qsort.txt
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Ensure comparison functions used by qsort(3) fulfill transitivity, since
otherwise the resulting array might not be sorted correctly or worse[1]
in case of integer overflows.
[1]: https://www.qualys.com/2024/01/30/qsort.txt
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Convert the only usage of the raw type struct level_datum to use the
typedef. Simplifies refactorizations on the type.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
In the internal function sepol_av_to_string(), use a dynamically
allocated buffer for the permission names of an access vector instead
of a fixed static buffer to support very long permission names.
Update the internal users of sepol_av_to_string() to free the buffer.
The exported function sepol_perm_to_string() is just a wrapper to
the internal function. To avoid changing the behavior of this function,
use a static buffer and copy the resulting string from the internal
function. If the string is too long for the buffer or there was an
error in creating the string, return a string indicating the error.
All of the changes to the internal function and users was the work
of Christian Göttsche <cgzones@googlemail.com>.
Reported-by: oss-fuzz (issue 64832, 64933)
Suggested-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
Pass LDFLAGS when checking for reallocarray to avoid the following
static build failure with musl raised since version 3.4 and
f0a5f6e330
because -static is not passed when checking for reallocarray:
/home/autobuild/autobuild/instance-9/output-1/host/bin/armeb-buildroot-linux-musleabi-gcc -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -O0 -g0 -static -I. -I../include -D_GNU_SOURCE -I../cil/include -fPIC -c -o assertion.o assertion.c
In file included from assertion.c:28:
private.h:88:21: error: static declaration of 'reallocarray' follows non-static declaration
88 | static inline void* reallocarray(void *ptr, size_t nmemb, size_t size) {
| ^~~~~~~~~~~~
In file included from ../include/sepol/policydb/mls_types.h:35,
from ../include/sepol/policydb/context.h:23,
from ../include/sepol/policydb/policydb.h:62,
from assertion.c:24:
/home/autobuild/autobuild/instance-9/output-1/host/armeb-buildroot-linux-musleabi/sysroot/usr/include/stdlib.h:150:7: note: previous declaration of 'reallocarray' with type 'void *(void *, size_t, size_t)' {aka 'void *(void *, unsigned int, unsigned int)'}
150 | void *reallocarray (void *, size_t, size_t);
| ^~~~~~~~~~~~
Fixes:
- http://autobuild.buildroot.org/results/0170032548a38e2c991d62dc5823808458ad03b3
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The canonical order of calloc(3) parameters is the number of elements
first and the size of each element second.
Reported by GCC 14:
kernel_to_conf.c:814:47: warning: 'calloc' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Wcalloc-transposed-args]
kernel_to_conf.c:945:46: warning: 'calloc' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Wcalloc-transposed-args]
kernel_to_conf.c:2109:35: warning: 'calloc' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Wcalloc-transposed-args]
kernel_to_common.c:578:29: warning: 'calloc' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Wcalloc-transposed-args]
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Validate no common classes inside scope indices are defined.
Reported-by: oss-fuzz (issue 64849)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Declare the read-only permission parameter const.
Use a more readable overflow check, which is also resilient against
changes of the growth factor or initial size.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The input string to be tokenized is not modified.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Commit fb0a4ce1 (libsepol/cil: Allow paths in filecon rules to be
passed as arguments) changed when the new AST node data would be set
to point to the new filecon struct when creating a filecon rule.
This causes cil_destroy_filecon() to be called twice on the filecon
struct if there is an error when creating the filecon rule.
If there is an error when creating a filecon rule, call
cil_clear_node() after destroying the filecon struct.
Reported-by: oss-fuzz (issue 64385)
Signed-off-by: James Carter <jwcart2@gmail.com>
This patch adds CPPFLAGS to all of the Makefiles as suggested.
Signed-off-by: Cameron Williams <ckwilliams.work@gmail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Simplify the string formatting helpers create_str() and
strs_create_and_add() by calling the GNU extension vasprintf(3), already
used in libsepol/cil/. This allows a redundant parameter from both
functions to be dropped.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Standard policy modules generated by compilers have at least one global
av rule. Reject modules otherwise, e.g. generated by a fuzzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Do not check assertions for policies without any av rules.
Only output kernel policies in traditional and CIL format.
Perform hierarchy constraint checks.
Try to link, expand and output base module policies.
Also rework argument passing of verbose flags to improve debugging
usability.
Reported-by: oss-fuzz (issues 64515, 64531)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Internally class values are stored in multiple placed in a 16-bit wide
integer. Reject class values exceeding the maximum representable value.
This avoids truncations in the helper
policydb_string_to_security_class(), which gets called before validation
of the policy:
policydb.c:4082:9: runtime error: implicit conversion from type 'uint32_t' (aka 'unsigned int') of value 2113929220 (32-bit, unsigned) to type 'sepol_security_class_t' (aka 'unsigned short') changed the value to 4 (16-bit, unsigned)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Vit Mojzis
Thiébaud Weksteen
James Carter
Petr Lautrbach
Christian Göttsche
Fabrice Fontaine
Cameron Williams