Using clang's static analyzer is as simple as running "scan-build make",
but in order to obtain clean and reproducible results, the build
environment has to be cleaned beforehand ("make clean distclean").
Moreover the project requires running "make install" before "make test"
in order to install the dependencies needed for the tests, and running
these tests with the newly-built libraries requires a specific
LD_LIBRARY_PATH. This new script takes care of setting up everything
which is needed.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When write_binary_policy() fails to open the binary policy, it calls
sepol_handle_destroy(f.handle) but structure f has not been initialized
at this point. Use variable handle instead.
This issue has been found using clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
f.handle is never set in read_binary_policy() so there is no need to
call sepol_handle_destroy() on it. Moreover clang's static analyzer
warns about an uninitialized argument value in the first call.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
va_copy()'s manpage [1] states:
Each invocation of va_copy() must be matched by a corresponding
invocation of va_end() in the same function.
create_str_helper() is using va_copy() without va_end(). Add the missing
call.
[1] https://linux.die.net/man/3/va_copy
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
require_class() allocate memory for its variable "class_datum_t *datum"
and calls symtab_init(&datum->permissions, PERM_SYMTAB_SIZE). If this
second call fails, datum is not freed.
Fix this memory leak.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
sepol_ppfile_to_module_package() does not use its variable "FILE *f =
NULL;" but to fclose() it. This variable has been unneeded since the
introduction of function ppfile_to_module_package() in commit
893851c0a1 ("policycoreutils: add a HLL compiler to convert policy
packages (.pp) to CIL").
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
clang's static analyzer reports that ebitmap_to_names() can call
malloc(0) when the bitmap is empty. If malloc() returns NULL, this
triggers a misleading "Out of memory" error.
Work around this by treating empty bitmaps as appropriate.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
clang's static analyzer warns about dead assignments to local variables.
In module_to_cil.c, there are some which are quite straightforward to
review. Remove them.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When list_prepend() returns an error, it always means it failed to
allocate some memory and does not hold any reference to its argument
data. This argument needs to be freed by the caller in order to prevent
a memory leak.
While reviewing list_prepend() callers, I spend quite some time
understanding why typealiases_gather_map() does not need to strdup(key)
or free(key) when calling list_prepend(..., key) even though "key" comes
from pdb->p_types.table: because typealias_list_destroy() does not free
the inserted items. Add a comment to make this clearer in the code.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In cil_tree_print_expr(), "rc < 0" is equivalent to "rc != 0" but
clang's static analyzer does not know about this. Help it.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Ruby 2.5 is not installed by default, force reinstall with rvm
Signed-off-by: Jason Zaman <jason@perfinion.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
from getpwnam_r(3): "The call sysconf(_SC_GETPW_R_SIZE_MAX) returns
either -1, without changing errno, or an initial suggested size for buf.
(If this size is too small, the call fails with ERANGE, in which case
the caller can retry with a larger buffer.)"
The same can happen for _SC_GETGR_R_SIZE_MAX. 1024 appears to be a good
fallback but may need revisiting in the future.
This triggered an error on musl libc but could happen other places too.
Signed-off-by: Jason Zaman <jason@perfinion.com>
musl doesn't implement GLOB_BRACE and GLOB_TILDE, so simply don't use
them there. This affects restorecond -u but braces are not used in the
example configs. GLOB_TILDE is on the roadmap[1] for musl 1.1.21 so
restorecond -u should be fine soon.
[1]: https://wiki.musl-libc.org/roadmap.html
Signed-off-by: Jason Zaman <jason@perfinion.com>
musl doesn't implement GLOB_BRACE and GLOB_TILDE, so simply don't use
them there. This only affects "setfiles -f", which I don't expect many
people use, and it's undocumented anyway that it expands globs.
Signed-off-by: Luis Ressel <aranea@aixah.de>
Signed-off-by: Jason Zaman <jason@perfinion.com>
Musl libc does not include the fts(3) functions so need to link to the
musl-fts library
https://github.com/pullmoll/musl-fts
Signed-off-by: Jason Zaman <jason@perfinion.com>
Fix the following ambiguous output (from booting with init=/bin/sh):
# /usr/sbin/fixfiles onboot
/usr/sbin/fixfiles: line 313: /.autorelabel: Read-only file system
/usr/sbin/fixfiles: line 317: /.autorelabel: Read-only file system
System will relabel on next boot
System will not relabel on next boot if we couldn't create ./autorelabel
(In case anyone reading this description is still confused: To run
`fixfiles onboot` after booting with init=/bin/sh, you must first run
`mount / -oremount,rw`).
Verify that the final path does not exceed the size of the
buffer before copying. This can only occur if an alternate
path for the policy root and/or the policy store root has been
specified and if the resulting path would exceed PATH_MAX. A
similar check is already applied by semanage_make_final().
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
self.store is always a string (actual store name or "") because of
semanageRecords.__init__. Fix check for not defined store.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1559174#c3
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Fix the following build warnings.
audit2why.c: In function ‘__policy_init’:
audit2why.c:207:22: warning: ‘%s’ directive output may be truncated writing up to 4095 bytes into a region of size 4081 [-Wformat-truncation=]
"unable to open %s: %s\n",
^~
path, strerror(errno));
~~~~
audit2why.c:206:4: note: ‘snprintf’ output 20 or more bytes (assuming 4115) into a destination of size 4096
snprintf(errormsg, sizeof(errormsg),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"unable to open %s: %s\n",
~~~~~~~~~~~~~~~~~~~~~~~~~~~
path, strerror(errno));
~~~~~~~~~~~~~~~~~~~~~~
audit2why.c:253:28: warning: ‘%s’ directive output may be truncated writing up to 4095 bytes into a region of size 4074 [-Wformat-truncation=]
"invalid binary policy %s\n", path);
^~ ~~~~
audit2why.c:252:3: note: ‘snprintf’ output between 24 and 4119 bytes into a destination of size 4096
snprintf(errormsg, sizeof(errormsg),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"invalid binary policy %s\n", path);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Fix the following warning in save_booleans(). We could likely drop
the function altogether, either ignoring or returning EINVAL if
a non-zero permanent argument is passed to security_set_boolean_list(),
since setting persistent booleans is now handled via libsemanage. This
code and the corresponding security_load_booleans() code is legacy from
RHEL4 days and could be removed although we would need to keep the ABI
for compatibility.
booleans.c: In function ‘save_booleans’:
booleans.c:441:13: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size between 0 and 8191 [-Werror=format-truncation=]
"%s=%d\n", boolname,
^~
booleans.c:440:7: note: ‘snprintf’ output between 4 and 8205 bytes into a destination of size 8192
snprintf(outbuf, sizeof(outbuf),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s=%d\n", boolname,
~~~~~~~~~~~~~~~~~~~~
boollist[i].value);
~~~~~~~~~~~~~~~~~~
booleans.c:454:12: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size between 0 and 8191 [-Werror=format-truncation=]
"%s=%d\n", boolname, val);
^~
booleans.c:453:6: note: ‘snprintf’ output between 4 and 8205 bytes into a destination of size 8192
snprintf(outbuf, sizeof(outbuf),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s=%d\n", boolname, val);
~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Fix the following build warning:
policydb.c: In function ‘get_symtab_destroy_func’:
policydb.c:1581:9: error: cast between incompatible function types from ‘int (*)(char *, void *, void *)’ to ‘void (*)(char *, void *, void *)’ [-Werror=cast-function-type]
return (hashtab_destroy_func_t) destroy_f[sym_num];
^
It turns out that this function and type are long unused in libsepol
and are not exported APIs for the shared library, so just remove them.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
When split_args() calls append_arg(), the returned value needs to be
checked in order to detect memory allocation failure. Checks were
missing in two places, which are spotted by clang's static analyzer:
semanage_store.c:1352:7: warning: Value stored to 'rc' is never
read
rc = append_arg(&argv, &num_args, arg);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
semanage_store.c:1368:3: warning: Value stored to 'rc' is never read
rc = append_arg(&argv, &num_args, arg);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
clang's static analyzer reports a potential memory leak because the
buffers allocated in pc and fc are not freed in main(), in sestatus.c.
Free these buffers properly.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
"sestatus -v" uses /proc/$PID/exe symbolic link in order to find the
context of processes present in /etc/sestatus.conf. For example, this
file includes "/usr/sbin/sshd".
On Arch Linux, /bin, /sbin and /usr/sbin are symbolic links to /usr/bin,
so sshd process is seen as "/usr/bin/sshd" instead of "/usr/sbin/sshd".
This causes "sestatus -v" to show nothing in "Process contexts:" for
sshd, agetty, etc.
Use realpath() to resolve any symlink components in program paths
defined in /etc/sestatus.conf. This makes "sestatus -v" show the
expected result:
Process contexts:
Current context: sysadm_u:sysadm_r:sysadm_t
Init context: system_u:system_r:init_t
/sbin/agetty system_u:system_r:getty_t
/usr/sbin/sshd system_u:system_r:sshd_t
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
This reverts commit 814631d3ae.
As reported by Petr Lautrbach, this commit changed the behavior
of selabel_open() when SELABEL_OPT_VALIDATE is 0, and this would
be an API change.
Reported-by: Petr Lautrbach <plautrba@redhat.com>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
In getconlist.c's main(), "level" is duplicated from an optional
argument without being ever freed. clang's static analyzer warns about
this memory leak.
Free the allocated memory properly in order to remove a warning reported
by clang's static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
If store_stem() fails to expand the memory allocated on data->stem_arr,
some things go wrong:
* the memory referenced by "buf" is leaked,
* data->alloc_stems has been increased without data->stem_arr having
been expanded. So the next time store_stem() is called, the function
will behave as if the buffer holds enough space, and will write data
after the end of data->stem_arr.
The first issue is being spotted by clang's static analyzer, which warns
about leaking variable "stem" in find_stem_from_spec() (this function
calls store_stem()).
This both issues by freeing buf when realloc(data->stem_arr) fails, and
by not increasing data->alloc_stems when this happens.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When load_users() parses an invalid line with an empty level context
(ie. nothing between "level" and "range" keywords), it allocates memory
with malloc(0) and uses it. The behavior of malloc() in this case is
an unspecified behavior: it might return NULL, which would lead to a
segmentation fault.
Fix this issue by reporting the invalid entry instead. While at it,
ensure that the character before "range" is a space, and change the
logic slightly in order to avoid using "--p; ... p++;".
This issue is reported by clang's static analyzer with the following
message:
genusers.c:222:11: warning: Use of zero-allocated memory
*r++ = *s;
^
genusers.c:225:7: warning: Use of zero-allocated memory
*r = 0;
^
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
In cond_expr_to_cil(), when stack_init() fails to allocate a stack, the
function calls stack_pop() with stack = NULL. Then stack_pop()
dereferences the pointer ("if (stack->pos == -1) {"), which is NULL.
Fix this by moving the stack cleaning loop in a "if (stack != NULL)"
block.
This issue is reported by clang's static analyzer with the following
message:
module_to_cil.c:463:6: warning: Access to field 'pos' results in a
dereference of a null pointer (loaded from variable 'stack')
if (stack->pos == -1) {
^~~~~~~~~~
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
This allows sepolgen to generate policy from AVC messages that contain
contexts translated by mcstrans.
Fixes:
\# echo "type=USER_AVC msg=audit(1468415802.940:2199604): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0-s15:c0.c1023 msg='avc: denied { status } for auid=n/a uid=0 gid=0 cmdline="/usr/lib/systemd/systemd-logind" scontext=system_u:system_r:systemd_logind_t:SystemLow-SystemHigh tcontext=system_u:system_r:init_t:s0-s15:c0.c1023 tclass=system exe="/usr/lib/systemd/systemd" sauid=0 hostname=? addr=? terminal=?'" | audit2allow
libsepol.mls_from_string: invalid MLS context SystemLow-SystemHigh
libsepol.mls_from_string: could not construct mls context structure
libsepol.context_from_record: could not create context structure
libsepol.context_from_string: could not create context structure
libsepol.sepol_context_to_sid: could not convert
system_u:system_r:systemd_logind_t:SystemLow-SystemHigh to sid
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Nicolas Iooss
Laurent Bigonville
Stephen Smalley
Jason Zaman
Luis Ressel
Alan Jenkins
Vit Mojzis