Using mount flag `nosuid` also affects SELinux domain transitions but
this has not been documented well.
Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
When generating CIL policy from kernel or module policy quote paths,
which are allowed to contain spaces, in the statements `genfscon` and
`devicetreecon`.
Reported by LuK1337 while building policy for Android via IRC.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Improve formatting of section DESCRIPTION by adding list points.
Mention errno is set on failure.
Mention the returned context might be NULL if SELinux is not enabled.
Align setcon/_raw parameter by adding const.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
In case of a recurring call to `selinux_status_open(3)`, which
previously has been opened in fallback mode, return `1` according to its
documentation.
Fixes: c5a699046f ("libselinux: make selinux_status_open(3) reentrant")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
Anonymous levels can be passed as call arguments and they can
appear in anonymous levelranges as well.
Anonymous call arguments are resolved when they are used in a rule.
If more than one rule uses the anonymous level, then a memory leak
will occur when a new list for the category datum expression is
created without destroying the old one.
When resolving a level, check if the sensitivity datum has already
been resolved. If it has, then the categories have been as well.
Signed-off-by: James Carter <jwcart2@gmail.com>
Set the pointer to the sensitivity in levels, the pointers to the low
and high levels in levelranges, the pointer to the level in userlevels,
the pointer to the range in userranges, and the pointers to contexts
in ocontexts to NULL.
Signed-off-by: James Carter <jwcart2@gmail.com>
Anonymous class permission sets can be passed as call arguments.
Anonymous call arguments are resolved when they are used in a
rule. [This is because all the information might not be present
(like common permissions being added to a class) when the call
itself is resolved.] If there is more than one rule using an
anonymous class permission set, then a memory leak will occur
when a new list for the permission datum expression is created
without destroying the old one.
When resolving the class and permissions, check if the class has
already been resolved. If it has, then the permissions have been
as well.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
When parsing a CIL policy, the number of open parenthesis is tracked
to verify that each has a matching close parenthesis. If there are
too many open parenthesis, a stack overflow could occur during later
processing.
Exit with an error if the number of open parenthesis exceeds 4096
(which should be enough for any policy.)
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
When exiting with an error because a class or common has too many
permissions, destroy the permission nodes.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
A failed tunable resolution in a tunableif can cause an optional
to be disabled before the CIL_PASS_CALL1 phase. If this occurs, the
optional block and its subtree should be destroyed, but no reset
will be required since tunables are not allowed inside an optional
block.
Anytime there are optional blocks in the disabled_optionals list
(changed == 1), destroy the optional block and its subtree even if
in a pass before CIL_PASS_CALL1.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
The listing of the order was in the macro section, but it belongs
in the call section.
Move the listing of the order to the call section and provide a
better explanation.
Signed-off-by: James Carter <jwcart2@gmail.com>
Lorenzo Ceragioli <lorenzo.ceragioli@phd.unipi.it> noted that the
following policy:
(type a)
(block A
(macro m ((type x))
(type a)
(allow x x (file (read))))
)
(block B
(call A.m(a))
)
results in the allow rule (allow B.a B.a (file(read))). This makes
no sense because the "a" being passed as an argument has to be the
global "a" and not the "a" defined in the macro.
This behavior occurs because the call arguments are resolved AFTER
the macro body has been copied and the declaration of "a" in the
macro has been added to block B's namespace, so this is the "a"
that the call argument resolves to, rather than the one in the
global namespace.
When resolving call arguments, check if the datum found belongs to
a declaration in the call. If it does, then remove the datum from
the symbol table, re-resolve the argument, and add the datum back
into the symbol table.
Signed-off-by: James Carter <jwcart2@gmail.com>
Rename cil_resolve_call1() as cil resolve_call() and rename
cil_resolve_call2() as cil_resolve_call_args() to make it clearer
what is being done in each function.
Move code to build call arguments out of cil_resolve_call() and into
the new function called cil_build_call_args() so that the logic of
cil_resolve_call() can be seen.
Exit cil_resolve_call() immediately if the call has already been
copied.
In __cil_resolve_ast_node(), only resolve calls outside of macros.
This results in more calls to cil_copy_ast(), but slightly less
rules copied overall (since no rules are copied into a macro). This
also means that the CIL_PASS_MACRO pass is not needed and can be
eliminated.
Signed-off-by: James Carter <jwcart2@gmail.com>
Allow inserting a key without providing a node.
This will make it easier to properly resolve call arguments where
a key might need to be temporarily removed to search for a datum
that is not declared within the call. Since the node is already
in the node list, re-inserting the key without this option would
add another link to the node and cause problems.
Also, do not add the node to the datum's node list if the result
of the call to hashtab_insert() is SEPOL_EEXIST because the datum
is a duplicate and will be destroyed.
Signed-off-by: James Carter <jwcart2@gmail.com>
The CIL Reference Guide specifies how name resolution is suppose
to work within an expanded macro.
1. Items defined inside the macro
2. Items passed into the macro as arguments
3. Items defined in the same namespace of the macro
4. Items defined in the caller's namespace
5. Items defined in the global namespace
But Lorenzo Ceragioli <lorenzo.ceragioli@phd.unipi.it> found
that the first step is not done.
So the following policy:
(block A
(type a)
(macro m ()
(type a)
(allow a self (CLASS (PERM)))
)
)
(block B
(call A.m)
)
will result in:
(allow A.a self (CLASS (PERM)))
instead of the expected:
(allow B.a self (CLASS (PERM)))
Now when an expanded call is found, the macro's namespace is
checked first. If the name is found, then the name was declared
in the macro and it is declared in the expanded call, so only the
namespace of the call up to and including the global namespace
will be searched. If the name is not found in the macro's namespace
then name resolution continues with steps 2-5 above.
Signed-off-by: James Carter <jwcart2@gmail.com>
Currently `avc_init_internal()`, called by `avc_open(3)` and
`avc_init(3)`, does open the SELinux status page with fallback mode
enabled.
Quote from man:selinux_status_open(3):
In this case, this function tries to open a netlink socket using
.BR avc_netlink_open (3) and overwrite corresponding callbacks
(setenforce and policyload). Thus, we need to pay attention to the
interaction with these interfaces, when fallback mode is enabled.
Calling `selinux_status_open` internally in fallback mode is bad, cause
it overrides callbacks from client applications or the internal
fallback-callbacks get overridden by client applications.
Note that `avc_open(3)` gets called under the hood by
`selinux_check_access(3)` without checking for failure.
Also the status page is available since Linux 2.6.37, so failures of
`selinux_status_open(3)` in non-fallback mode should only be caused by
policies not allowing the client process to open/read/map
the /sys/fs/selinux/status file.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not mmap the status page again if `selinux_status_open(3)` has already
been called with success.
`selinux_status_open(3)` might be called unintentionally multiple times,
e.g. once to manually be able to call `selinux_status_getenforce(3)` and
once indirectly through `selinux_check_access(3)`
(since libselinux 3.2).
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Mention in the manpage of avc_destroy(3) that it does close the SELinux
status page, which might have been opened manually by the client
application.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
In the blockinherit section of the CIL documentation clearly state
the order in which inherited rules are resolved.
That order is:
1) The parent namespaces (if any) where the blockinherit rule is
located with the exception of the global namespace.
2) The parent namespaces of the block being inherited (but not that
block's namespace) with the exception of the global namespace.
3) The global namespace.
Signed-off-by: James Carter <jwcart2@gmail.com>
When resolving a name in a block that has been inherited. First,
a search is done in the parent namespaces (if any) of the
blockinherit rule with the exception of the global namespace. If
the name is not found, then a search is done in the namespaces of
the original block (starting with that block's namespace) with
the exception of the global namespace. Finally, if it still has
not been found, the global namespace is searched.
This does not work if a declaration is in the block being
inherited.
For example:
(block b
(typeattribute a)
(allow a self (CLASS (PERM)))
)
(blockinherit b)
This will result in a policy with the following identical allow
rules:
(allow b.a self (CLASS (PERM)))
(allow b.a self (CLASS (PERM)))
rather than the expected:
(allow b.a self (CLASS (PERM)))
(allow a self (CLASS (PERM)))
This is because when the typeattribute is copied while resolving
the inheritance, the new datum is added to the global namespace
and, since that is searched last, the typeattribute in block b is
found first.
This behavior means that no declaration that is inherited into the
global namespace will actually be used.
Instead, if the name is not found in the parent namespaces (if any)
where the blockinherit is located with the exception of the global
namespace, start the next search in the namespace of the parent of
the original block (instead of the original block itself). Now if
a declaration is inherited from the original block, the new
declaration will be used. This behavior seems to be the originally
intended behavior because there is a comment in the code that says,
"Continue search in original block's parent".
This issue was found by secilc-fuzzer. If the original block
is made to be abstract, then the type attribute declaration
in the original block is not in the policy and a segfault
occurs when creating the binary because the copied allow rule
refers to a non-existent type attribute.
Signed-off-by: James Carter <jwcart2@gmail.com>
Found by clang-tidy.
libselinux/src/label_file.c:374:4: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
else
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Found by clang-tidy.
libselinux/src/avc_sidtab.h:32:6: warning: function 'sidtab_sid_stats' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
void sidtab_sid_stats(struct sidtab *s, char *buf, int buflen) ;
^
libselinux/src/avc_sidtab.c:103:6: note: the definition seen here
void sidtab_sid_stats(struct sidtab *h, char *buf, int buflen)
^
libselinux/src/avc_sidtab.h:32:6: note: differing parameters are named here: ('s'), in definition: ('h')
void sidtab_sid_stats(struct sidtab *s, char *buf, int buflen) ;
^ ~
h
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Open the file stream with the `e` flag, so that the underlying file
descriptor gets closed on an exec in a potential sibling thread.
Also drop the flag `b`, since it is ignored on POSIX systems.
Found by clang-tidy.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
In case `realloc()` fails and returns NULL, free the passed array,
instead of just setting the size helper variables to 0.
Also free the string contents in `free_array_elts()` of the array
`con_array`, instead of just the array of pointers.
Found by cppcheck.
src/matchpathcon.c:86:4: error: Common realloc mistake: 'con_array' nulled but not freed upon failure [memleakOnRealloc]
con_array = (char **)realloc(con_array, sizeof(char*) *
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
If any of the build flags `BUILD_HOST` or `ANDROID` is set and the
caller did not pass an option of type `SELABEL_OPT_PATH`, the variable
`path` might be not set.
Add a check to avoid calling `strdup()` with a NULL pointer.
Found by cppcheck.
src/label_file.c:759:26: warning: Possible null pointer dereference: path [nullPointer]
rec->spec_file = strdup(path);
^
src/label_file.c:713:21: note: Assignment 'path=NULL', assigned value is 0
const char *path = NULL;
^
src/label_file.c:759:26: note: Null pointer dereference
rec->spec_file = strdup(path);
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Found by Infer.
selinux_config.c:181: error: Resource Leak
resource of type `_IO_FILE` acquired by call to `fopen()` at line 165, column 7 is not released after line 181, column 6.
179. type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
180. if (!type)
181. return;
^
182. end = type + strlen(type) - 1;
183. while ((end > type) &&
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Found by Infer.
matchmediacon.c:25: error: Resource Leak
resource of type `_IO_FILE` acquired to `return` by call to `fopen()` at line 21, column 16 is not released after line 25, column 4.
23. while (!feof_unlocked(infile)) {
24. if (!fgets_unlocked(current_line, sizeof(current_line), infile)) {
25. return -1;
^
26. }
27. if (current_line[strlen(current_line) - 1])
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
GCC 11 complains:
In file included from label_file.c:24:
In function ‘store_stem’,
inlined from ‘load_mmap’ at label_file.c:277:12,
inlined from ‘process_file’ at label_file.c:551:5:
label_file.h:289:25: error: ‘free’ called on pointer ‘*mmap_area.next_addr’ with nonzero offset 4 [-Werror=free-nonheap-object]
289 | free(buf);
| ^~~~~~~~~
Free the pointer on failure at the caller instead of inside `store_stem()`.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not leak memory if program arguments get specified more than once.
Found by clang-anlyzer.
getdefaultcon.c:52:3: warning: Potential leak of memory pointed to by 'level' [unix.Malloc]
fprintf(stderr,
^~~~~~~~~~~~~~~
getdefaultcon.c:52:3: warning: Potential leak of memory pointed to by 'role' [unix.Malloc]
fprintf(stderr,
^~~~~~~~~~~~~~~
getdefaultcon.c:52:3: warning: Potential leak of memory pointed to by 'service' [unix.Malloc]
fprintf(stderr,
^~~~~~~~~~~~~~~
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `rc` is always unconditionally assigned by the next call of
`setexeccon()` and never read in between.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `lineno` is only used in the preceding loop and it always
set prior that to 0.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `lineno` is only used in the preceding loop and is always
set prior that to 0.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `i` is not used inside this loop, and it later
unconditionally set to 0.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not leak memory if the program argument `l` got passed more than
once.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Free all memory from `selabel_get_digests_all_partial_matches()` in case
of success and failure.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `dir_xattr_list` is only used inside `selinux_restorecon.c`.
selinux_restorecon.c:65:19: warning: no previous extern declaration for non-static variable 'dir_xattr_list' [-Wmissing-variable-declarations]
struct dir_xattr *dir_xattr_list;
^
selinux_restorecon.c:65:1: note: declare 'static' if the variable is not intended to be used outside of this translation unit
struct dir_xattr *dir_xattr_list;
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The format width specifier `L` is only standardized for floating point
types. Use `ll` for fixed-width data types.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Mark the argument `Buffer` of `Sha1Update()` const, since it is not
modified.
sha1.c: In function ‘Sha1Finalise’:
sha1.c:208:25: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
208 | Sha1Update(Context, (uint8_t*)"\x80", 1);
| ^
sha1.c:211:29: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
211 | Sha1Update(Context, (uint8_t*)"\0", 1);
| ^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
As the const qualifier is discarded in label_common(), do not return a
const qualified pointer pointer from the local function `lookup_all()`.
label_file.c: In function ‘lookup_common’:
label_file.c:994:24: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
994 | struct spec *result = (struct spec*)matches[0];
| ^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not discard the const qualifier of the function argument, and drop
the redundant local variable `keyp`.
avc_sidtab.c: In function ‘sidtab_hash’:
avc_sidtab.c:23:9: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
23 | keyp = (char *)key;
| ^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Now that F34 has been released, it's time to update the CI Vagrantfile
to use the new Fedora version. This also fixes the failure in the
recently added vsock_socket test that depends on a bugfix, which made it
to the F34 image's kernel, but is not in the F33 image's.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
The secilc-fuzzer found a self-referential loop using category sets.
Any set declaration in CIL that allows sets in it is susceptible to
the creation of a self-referential loop. There is a check, but only
for the name of the set being declared being used in the set
declaration.
Check for self-refential loops in user, role, and type attributes
and in category sets. Since all of the sets need to be declared,
this check has to be done when verifying the CIL db before doing
the post phase.
Signed-off-by: James Carter <jwcart2@gmail.com>
I can't think of a good reason why they should be excluded. On the
contrary, excluding them can cause trouble very easily if some labeling
rules for these directories change. For example, we changed the label
for /dev/nvme* from nvme_device_t to fixed_disk_device_t in Fedora
(updating the allow rules accordingly) and after policy update they
ended up with an invalid context, causing denials.
Thus, remove /dev and /run from the excludes. While there, also add
/root to the basic excludes to match the regex that excludes fc rules
(that should be effectively no functional change).
I did a sanity check on my system by running `restorecon -nv /dev /run`
and it didn't report any label differences.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
When building libselinux on Fedora 33 with gcc 10.3.1, the compiler
reports:
label_file.c: In function ‘lookup_all.isra’:
label_file.c:940:4: error: ‘strncpy’ specified bound depends on the
length of the source argument [-Werror=stringop-overflow=]
940 | strncpy(clean_key, key, len - 1);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
label_file.c:927:8: note: length computed here
927 | len = strlen(key);
| ^~~~~~~~~~~
cc1: all warnings being treated as errors
As clean_key is the result of malloc(len), there is no issue here. But
using strncpy can be considered as strange, because the size of the
string is already known and the NUL terminator is always added later, in
function ‘lookup_all.isra.
Replace strncpy with memcpy to silence this gcc false-positive warning.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
