By default, GIO channels use UTF-8 as encoding, which causes issues when
reading binary data such as inotify events.
Signed-off-by: Fabian Vogt <fvogt@suse.de>
Acked-by: James Carter <jwcart2@gmail.com>
When a user is created in an optional block, a user datum is added
to both the avrule_decl's symtab and the policydb's symtab, but
the semantic MLS information is only added to the avrule_decl's
user datum. This causes an error to occur during policy expansion
when user_copy_callback() is called. If this error did not occur
then the policydb's user datum would be written without any MLS
info and the policy would fail validation when read later.
When creating a user datum, search for a user datum with the same
key in the policydb's symtab. If that datum has no MLS information,
then copy the MLS information from the avrule_decl's datum. If it
does, then compare the default level, low level, and high level
sensitivities and give an error if they do not match. There is not
enough information to expand the categories for the high and low
levels, so merge the semantic categories. If the two category sets
are not equal an error will occur during the expansion phase.
A minimum policy to demonstrate the bug:
class CLASS1
sid kernel
class CLASS1 { PERM1 }
sensitivity SENS1;
dominance { SENS1 }
level SENS1;
mlsconstrain CLASS1 { PERM1 } ((h1 dom h2) and (l1 domby h1));
type TYPE1;
allow TYPE1 self : CLASS1 PERM1;
role ROLE1;
role ROLE1 types TYPE1;
optional {
require {
role ROLE1;
}
user USER2 roles ROLE1 level SENS1 range SENS1;
}
user USER1 roles ROLE1 level SENS1 range SENS1;
sid kernel USER1:ROLE1:TYPE1:SENS1
Signed-off-by: James Carter <jwcart2@gmail.com>
The following statement is always true if read_len is unsigned:
(read_len = read(fd, data_read + data_read_len, max_len - data_read_len)) > 0
Fixes:
Error: INTEGER_OVERFLOW (CWE-190): [#def19] [important]
libsemanage-3.7/src/direct_api.c:598:2: tainted_data_return: Called function "read(fd, data_read + data_read_len, max_len - data_read_len)", and a possible return value may be less than zero.
libsemanage-3.7/src/direct_api.c:598:2: cast_underflow: An assign of a possibly negative number to an unsigned type, which might trigger an underflow.
libsemanage-3.7/src/direct_api.c:599:3: overflow: The expression "data_read_len += read_len" is deemed underflowed because at least one of its arguments has underflowed.
libsemanage-3.7/src/direct_api.c:598:2: overflow: The expression "max_len - data_read_len" is deemed underflowed because at least one of its arguments has underflowed.
libsemanage-3.7/src/direct_api.c:598:2: overflow_sink: "max_len - data_read_len", which might have underflowed, is passed to "read(fd, data_read + data_read_len, max_len - data_read_len)". [Note: The source code implementation of the function has been overridden by a builtin model.]
\# 596| }
\# 597|
\# 598|-> while ((read_len = read(fd, data_read + data_read_len, max_len - data_read_len)) > 0) {
\# 599| data_read_len += read_len;
\# 600| if (data_read_len == max_len) {
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
Fixes:
Error: RESOURCE_LEAK (CWE-772):
libselinux-3.6/src/matchpathcon.c:519: alloc_arg: "lgetfilecon_raw" allocates memory that is stored into "con". [Note: The source code implementation of the function has been overridden by a user model.]
libselinux-3.6/src/matchpathcon.c:528: leaked_storage: Variable "con" going out of scope leaks the storage it points to.
\# 526|
\# 527| if (!hnd && (matchpathcon_init_prefix(NULL, NULL) < 0))
\# 528|-> return -1;
\# 529|
\# 530| if (selabel_lookup_raw(hnd, &fcontext, path, mode) != 0) {
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
Fixes:
Error: IDENTICAL_BRANCHES (CWE-398):
libselinux-3.6/src/setexecfilecon.c:45: implicit_else: The code from the above if-then branch is identical to the code after the if statement.
libselinux-3.6/src/setexecfilecon.c:43: identical_branches: The same code is executed when the condition "rc < 0" is true or false, because the code in the if-then branch and after the if statement is identical. Should the if statement be removed?
\# 41|
\# 42| rc = setexeccon(newcon);
\# 43|-> if (rc < 0)
\# 44| goto out;
\# 45| out:
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-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>
Set errno in open_file() if rolling_append(), which does not set errno,
failed, since transitive callers might rely on it.
Reported-by: clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Use fseek(3) instead of rewind(3) to detect failures.
Reported-by: clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Free the allocated line if it fails to parse via process_line() for the
X or media database.
Also declare the line_buf parameter of process_line() const, so it is
more obvious it is not modified or free'd.
Reported-by: clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
In case fclose(3) might modify the global variable errno, use a wrapper
retaining the errno value. In the affected cases the success of
fclose(3) itself is not important, since the underlying descriptor is
only read from.
Reported-by: clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
https://github.com/swig/swig/blob/master/CHANGES.current
"[Python] #2907 Fix returning null from functions with output
parameters. Ensures OUTPUT and INOUT typemaps are handled
consistently wrt return type.
New declaration of SWIG_Python_AppendOutput is now:
SWIG_Python_AppendOutput(PyObject* result, PyObject* obj, int is_void);
The 3rd parameter is new and the new $isvoid special variable
should be passed to it, indicating whether or not the wrapped
function returns void.
Also consider replacing with:
SWIG_AppendOutput(PyObject* result, PyObject* obj);
which calls SWIG_Python_AppendOutput with same parameters but adding $isvoid
for final parameter."
Fixes: https://github.com/SELinuxProject/selinux/issues/447
Suggested-by: Jitka Plesnikova <jplesnik@redhat.com>
Signed-off-by: Petr Lautrbach <lautrbach@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
https://github.com/swig/swig/blob/master/CHANGES.current
"[Python] #2907 Fix returning null from functions with output
parameters. Ensures OUTPUT and INOUT typemaps are handled
consistently wrt return type.
New declaration of SWIG_Python_AppendOutput is now:
SWIG_Python_AppendOutput(PyObject* result, PyObject* obj, int is_void);
The 3rd parameter is new and the new $isvoid special variable
should be passed to it, indicating whether or not the wrapped
function returns void.
Also consider replacing with:
SWIG_AppendOutput(PyObject* result, PyObject* obj);
which calls SWIG_Python_AppendOutput with same parameters but adding $isvoid
for final parameter."
Fixes: https://github.com/SELinuxProject/selinux/issues/447
selinuxswig_python_wrap.c: In function ‘_wrap_security_compute_user’:
selinuxswig_python_wrap.c:11499:17: error: too few arguments to function ‘SWIG_Python_AppendOutput’
11499 | resultobj = SWIG_Python_AppendOutput(resultobj, plist);
| ^~~~~~~~~~~~~~~~~~~~~~~~
selinuxswig_python_wrap.c:1248:1: note: declared here
1248 | SWIG_Python_AppendOutput(PyObject* result, PyObject* obj, int is_void) {
| ^~~~~~~~~~~~~~~~~~~~~~~~
selinuxswig_python_wrap.c: In function ‘_wrap_security_compute_user_raw’:
selinuxswig_python_wrap.c:11570:17: error: too few arguments to function ‘SWIG_Python_AppendOutput’
11570 | resultobj = SWIG_Python_AppendOutput(resultobj, plist);
| ^~~~~~~~~~~~~~~~~~~~~~~~
selinuxswig_python_wrap.c:1248:1: note: declared here
1248 | SWIG_Python_AppendOutput(PyObject* result, PyObject* obj, int is_void) {
| ^~~~~~~~~~~~~~~~~~~~~~~~
selinuxswig_python_wrap.c: In function ‘_wrap_security_get_boolean_names’:
selinuxswig_python_wrap.c:12470:17: error: too few arguments to function ‘SWIG_Python_AppendOutput’
12470 | resultobj = SWIG_Python_AppendOutput(resultobj, list);
| ^~~~~~~~~~~~~~~~~~~~~~~~
selinuxswig_python_wrap.c:1248:1: note: declared here
1248 | SWIG_Python_AppendOutput(PyObject* result, PyObject* obj, int is_void) {
| ^~~~~~~~~~~~~~~~~~~~~~~~
error: command '/usr/bin/gcc' failed with exit code 1
Suggested-by: Jitka Plesnikova <jplesnik@redhat.com>
Signed-off-by: Petr Lautrbach <lautrbach@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
It was originally marked for deprecation back in Feb 2020,
commit a41dfeb55d ("libselinux: deprecate security_compute_user(),
update man pages"), but the attribute was not added at the time.
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Acked-by: Petr Lautrbach <lautrbach@redhat.com>
In commit d95bc8b755 ("libselinux: migrating hashtab from
policycoreutils") and commit 4a420508a9 ("libselinux: adapting hashtab
to libselinux"), the hashtab implementation was copied to libselinux.
Since the same functions exist in libsepol (e.g., hashtab_create,
hashtab_destroy, etc), a compilation error is raised when both libraries
are included statically.
Prefix the libselinux internal implementation with "selinux_".
Signed-off-by: Thiébaud Weksteen <tweek@google.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 ioctl extended permission structures and functions can be reused for
other extended permissions. Use the more generic term "xperm" instead of
"ioctl".
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 explanatory comments follow the common style of comments (Classic language / CIL)
Signed-off-by: Dmitry Sharshakov <dmitry.sharshakov@siderolabs.com>
Acked-by: James Carter <jwcart2@gmail.com>
Avoid errors when adding comments to CIL output like in audit2allow
Signed-off-by: Dmitry Sharshakov <dmitry.sharshakov@siderolabs.com>
Acked-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>
The runtime disable functionality has been removed in Linux 6.4. Thus
security_disable(3) will no longer work on these kernels.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Trying to compile libselinux for 32-bit produces the following error:
selinux_restorecon.c:1194:31: error: comparison of integer expressions of different signedness: ‘__fsword_t’ {aka ‘int’} and ‘unsigned int’ [-Werror=sign-compare]
1194 | if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
| ^~
Since RAMFS_MAGIC = 0x858458f6 == 2240043254, which > 2^31, but < 2^32,
cast both as uint32_t for the comparison.
Reported-by: Daniel Schepler
Signed-off-by: James Carter <jwcart2@gmail.com>
Reviewed-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
The lower 64 bits of the subnet prefix for an ibpkeycon rule should
all be 0's. Unfortunately the check uses the s6_addr macro which refers
to the 16 entry array of 8-bit values in the union and does not refer
to the correct bits.
Use the s6_addr32 macro instead which refers to the 4 entry array of
32-bit values in the union and refers to the lower 64 bits.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
restorecon.h uses types defined in label.h, so it needs to include
label.h (or code using restorecon.h also needs to include label.h,
which is not practical).
Fixes:
$ make DESTDIR=~/obj install > make.out
In file included from semanage_store.c:39:
/home/sdsmall/obj/usr/include/selinux/restorecon.h:137:52: error:
‘struct selabel_handle’ declared inside parameter list will not be
visible outside of this definition or declaration [-Werror]
137 | extern void selinux_restorecon_set_sehandle(struct
selabel_handle *hndl);
| ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [Makefile:111: semanage_store.o] Error 1
make[1]: *** [Makefile:15: install] Error 2
make: *** [Makefile:40: install] Error 1
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Fixes segfault in selabel_open() on systems with SELinux disabled and without any
SELinux policy installed introduced by commit 5876aca048 ("libselinux: free
data on selabel open failure"):
$ sestatus
SELinux status: disabled
$ cat /etc/selinux/config
cat: /etc/selinux/config: No such file or directory
$ matchpathcon /abc
[1] 907999 segmentation fault (core dumped) matchpathcon /abc
Signed-off-by: Petr Lautrbach <lautrbach@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>
The option array passed to avc_open(3) is only read from.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.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>
Reject multiple nodecon declarations with the same address and netmask.
Avoids mistakes when defining them in different places or using both the
address-with-netmask and CIDR-notation syntax.
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>
See https://github.com/koalaman/shellcheck/issues/2769
Fixes:
$ shellcheck -S error fixfiles
In fixfiles line 189:
# These two sorts need to be separate commands \
^-- SC1143 (error): This backslash is part of a comment and does not continue the line.
For more information:
https://www.shellcheck.net/wiki/SC1143 -- This backslash is part of a comme...
Signed-off-by: Petr Lautrbach <lautrbach@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
Free constraint, like in all other error branches.
Reported-by: Cppcheck
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Since commit 5876aca0 ("libselinux: free data on selabel open failure")
the close handler of label backends must support partial initialized
state, e.g. ->data being NULL. Thus checks for NULL were added, but in
two cases the pointers in question were already dereferenced before.
Reorder the dereference after the NULL-checks.
Fixes: 5876aca0 ("libselinux: free data on selabel open failure")
Reported-by: Cppcheck
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
In case scandir(3) finds no entries still free the returned result to
avoid leaking it.
Also do not override errno in case of a failure.
Reported.by: Cppcheck
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
