Use struct initialization with designators to skip unnecessary memset(3)
calls. Since libsepol is not a security boundary uninitialized padding
is not a concern.
Also drop the dead assignment of a region to be free'd in the next line.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
If CFLAGS set by the user contains the warnings override
`-Wno-error=implicit-function-declaration` the availability check does
not work properly. Explicitly enable and treat this warnings as failure
by appending the appropriate flag.
Also include CPPFLAGS in the check.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Add some tests to verify assertion checking works for extended
permissions in conditional policies.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Indent the printed allow rule that triggered an assertion by two spaces
to improve readability.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Add support for extended permission rules in conditional policies.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Add support for extended permission rules in conditional policies by
adding a new policy version and adjusting writing and validating
policies accordingly.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Use const parameters where applicable to signal immutability.
Rename the passed iterator avrule from avrule to narule, to make clear
its the neverallow rule to assert against, not the allow rule to check.
Drop needless branch in check_assertions(), since in the case avrules is
NULL the for loop won't execute and errors will stay at 0, so 0 will be
returned regardless. Also there is no call to free() as mentioned in the
outdated comment.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Certain roles (user_r, staff_r, sysadm_r, system_r, unconfined_r,
auditadm_r, and secadm_r) have always been handled in a special
way when converting a policy module to CIL to avoid having
duplicate role declarations. By optionally allowing duplicate role
declarations in CIL and by creating an option in libsemanage to
make use of duplicate declaration support, the special handling of
these roles can be removed.
Remove the special handling of certain roles in module_to_cil.c.
Signed-off-by: James Carter <jwcart2@gmail.com>
Allow duplicate role declarations (along with duplicate type and
type attribute declarations and context rules) if the multiple_decls
field in the CIL db has been set. This field can be set by a call to
cil_set_multiple_decls().
Signed-off-by: James Carter <jwcart2@gmail.com>
commit ba7945a250 added support for nlmsg extended permissions in the
policy. The assertion validation was not updated which lead to false
positives when evaluated. The optimization update was also missing. Add
support for the new extended permission for optimization and assertions.
Fixes: ba7945a250
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
Acked-by: James Carter <jwcart2@gmail.com>
The value of "strs" was not always initialized before being used by
strs_destroy.
Fixes:
Error: UNINIT (CWE-457):
libsepol-3.7/src/kernel_to_cil.c:1439:2: var_decl: Declaring variable "strs" without initializer.
libsepol-3.7/src/kernel_to_cil.c:1487:2: uninit_use_in_call: Using uninitialized value "strs" when calling "strs_destroy".
\# 1485|
\# 1486| exit:
\# 1487|-> strs_destroy(&strs);
\# 1488|
\# 1489| if (rc != 0) {
Error: UNINIT (CWE-457):
libsepol-3.7/src/kernel_to_conf.c:1422:2: var_decl: Declaring variable "strs" without initializer.
libsepol-3.7/src/kernel_to_conf.c:1461:2: uninit_use_in_call: Using uninitialized value "strs" when calling "strs_destroy".
\# 1459|
\# 1460| exit:
\# 1461|-> strs_destroy(&strs);
\# 1462|
\# 1463| if (rc != 0) {
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
tmp.node was not always initialized before being used by
ebitmap_destroy.
Fixes:
Error: UNINIT (CWE-457):
libsepol-3.7/cil/src/cil_post.c:1309:2: var_decl: Declaring variable "tmp" without initializer.
libsepol-3.7/cil/src/cil_post.c:1382:6: uninit_use_in_call: Using uninitialized value "tmp.node" when calling "ebitmap_destroy".
\# 1380| if (rc != SEPOL_OK) {
\# 1381| cil_log(CIL_INFO, "Failed to apply operator to bitmaps\n");
\# 1382|-> ebitmap_destroy(&tmp);
\# 1383| goto exit;
\# 1384| }
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
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>
