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>
Avoid truncations of the read 32 bit unsigned integer:
conditional.c:764:8: runtime error: implicit conversion from type 'uint32_t' (aka 'unsigned int') of value 3758096384 (32-bit, unsigned) to type 'int' changed the value to -536870912 (32-bit, signed)
conditional.c:831:8: runtime error: implicit conversion from type 'uint32_t' (aka 'unsigned int') of value 4280295456 (32-bit, unsigned) to type 'int' changed the value to -14671840 (32-bit, signed)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Similar to unconditional avtab keys check the default type of type av
rules are a simple type, not an attribute.
Since extended permission rules are not allowed in conditional policies
this check does not need to be performed.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
An extended access vector rule can consist of many individual ranges of
permissions. Use a dynamically growing sized buffer for formatting such
rules instead of a static buffer to avoid write failures due to
truncations.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
An extended access vector rule can consist of many individual ranges of
permissions. Use a dynamically growing sized buffer for formatting such
rules instead of a static buffer to avoid write failures due to
truncations.
Reported-by: oss-fuzz (issue 64316)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
In mls_semantic_level_expand(), there is a explicitly determine
whether category is 0, which may cause an potential integer
overflow in error branch.
Signed-off-by: Huaxin Lu <luhuaxin1@huawei.com>
Acked-by: James Carter <jwcart2@gmail.com>
Macros can use classpermission arguments. These are used in two
different ways. Either a named classpermission is passed (which is
declared using a classpermisison rule) or an anonymous classpermission
is passed (something like "(CLASS (PERM))").
Usually this will look like either of the following:
Ex1/
(classpermission cp1)
(classpermisisonset cp1 (CLASS (PERM)))
(macro m1 ((classpermisison ARG1))
(allow t1 self ARG1)
)
(call m1 (cp1))
or
Ex2/
(macro m2 ((classpermission ARG2))
(allow t2 self ARG2)
)
(call m2 ((CLASS (PERM))))
The following would also be valid:
Ex3/
(classpermission cp3)
(macro m3 ((classpermission ARG3))
(classpermissionset ARG3 (CLASS (PERM)))
(allow t3 self ARG3)
)
(call m3 (cp3))
The oss-fuzzer did the equivalent of the following:
(classpermission cp4)
(macro m4 ((classpermission ARG4))
(classpermissionset ARG4 (CLASS (PERM1)))
(allow t4 self ARG4)
)
(call m4 (CLASS (PERM2)))
It passed an anonymous classpermission into a macro where there
was a classpermissionset rule. Suprisingly, everything worked well
until it was time to destroy the AST. There is no way to distinguish
between the anonymous classpermission being passed in which needs
to be destroyed and the classpermission in the classpermissionset
rule which is destroyed when the classpermissionset rule is
destroyed. This led to CIL trying to destroy the classpermission
in the classpermissionset rule twice.
To fix this, when resolving the classpermission name in the
classpermissionset rule, check if the datum returned is for
an anonymous classpermission (it has no name) and return an
error if it is.
This fixes oss-fuzz issue 60670.
Signed-off-by: James Carter <jwcart2@gmail.com>
While still giving an error if there is a declaration with the
same flavor and name as a macro parameter, now give a warning in
the case where there is a declaration with the same name as a
macro parameter, but with a different flavor.
Example/
(macro m1 ((string ARG1))
(type ARG1)
(allow ARG1 ARG1 (CLASS (PERM)))
(typetransition t1a t1b CLASS ARG1 t1c)
)
(call m1 (foo))
This will result in the following equivalent code:
(type ARG1)
(allow ARG1 ARG1 (CLASS (PERM)))
(typetransition t1a t1b CLASS "foo" t1c)
With the warning (if using "-v"), "Declaration of type ARG1 has
same name as a macro parameter with a different flavor"
Signed-off-by: James Carter <jwcart2@gmail.com>
