Commit graph

3104 commits

Author SHA1 Message Date
Nicolas Iooss
c5e6153720
libsepol/cil: fix NULL pointer dereference in __cil_insert_name
OSS-Fuzz found a Null-dereference in __cil_insert_name when trying to
compile the following policy:

    (macro MACRO ()
        (classmap CLASS (PERM))
        (type TYPE)
        (typetransition TYPE TYPE CLASS "name" TYPE)
    )
    (call MACRO)

When using a macro with no argument, macro->params is NULL and
cil_list_for_each(item, macro->params) dereferenced a NULL pointer.
Fix this by checking that macro->params is not NULL before using it.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28565
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-03-17 08:46:36 +01:00
Nicolas Iooss
68e8871cfc
libsepol/cil: replace printf with proper cil_tree_log
All functions of the CIL compiler use cil_log or cil_tree_log to report
errors, but in two places which still uses printf. Replace these printf
invocation with cil_tree_log.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-03-17 08:46:35 +01:00
Nicolas Iooss
fba672edfb
libsepol/cil: remove stray printf
printf("%i\n", node->flavor); looks very much like a statement which was
added for debugging purpose and was unintentionally left.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-03-17 08:46:34 +01:00
Nicolas Iooss
ba5fb7a41b
libsepol/cil: make cil_post_fc_fill_data static
cil_post_fc_fill_data() is not used outside of cil_post.c, and is not
exported in libsepol.so. Make it static, in order to ease the analysis
of static analyzers.

While at it, make its path argument "const char*" and the fields of
"struct fc_data" "unsigned int" or "size_t", in order to make the types
better match the values.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-03-17 08:46:33 +01:00
James Carter
43c5ed469c
libsepol: Check kernel to CIL and Conf functions for supported versions
For policy versions between 20 and 23, attributes exist in the
policy, but only in the type_attr_map. This means that there are
gaps in both the type_val_to_struct and p_type_val_to_name arrays
and policy rules can refer to those gaps which can lead to NULL
dereferences when using sepol_kernel_policydb_to_conf() and
sepol_kernel_policydb_to_cil().

This can be seen with the following policy:
  class CLASS1
  sid SID1
  class CLASS1 { PERM1 }
  attribute TYPE_ATTR1;
  type TYPE1;
  typeattribute TYPE1 TYPE_ATTR1;
  allow TYPE_ATTR1 self : CLASS1 PERM1;
  role ROLE1;
  role ROLE1 types TYPE1;
  user USER1 roles ROLE1;
  sid SID1 USER1:ROLE1:TYPE1

Compile the policy:
  checkpolicy -c 23 -o policy.bin policy.conf
Converting back to a policy.conf causes a segfault:
  checkpolicy -F -b -o policy.bin.conf policy.bin

Have both sepol_kernel_policydb_to_conf() and
sepol_kernel_policydb_to_cil() exit with an error if the kernel
policy version is between 20 and 23.

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
2021-03-15 21:52:04 +01:00
James Carter
750cc1136d
checkpolicy: Do not automatically upgrade when using "-b" flag
When reading a binary policy, do not automatically change the version
to the max policy version supported by libsepol or, if specified, the
value given using the "-c" flag.

If the binary policy version is less than or equal to version 23
(POLICYDB_VERSION_PERMISSIVE) than do not automatically upgrade the
policy and if a policy version is specified by the "-c" flag, only set
the binary policy to the specified version if it is lower than the
current version.

If the binary policy version is greater than version 23 than it should
be set to the maximum version supported by libsepol or, if specified,
the value given by the "-c" flag.

The reason for this change is that policy versions 20
(POLICYDB_VERSION_AVTAB) to 23 have a more primitive support for type
attributes where the datums are not written out, but they exist in the
type_attr_map. This means that when the binary policy is read by
libsepol, there will be gaps in the type_val_to_struct and
p_type_val_to_name arrays and policy rules can refer to those gaps.
Certain libsepol functions like sepol_kernel_policydb_to_conf() and
sepol_kernel_policydb_to_cil() do not support this behavior and need
to be able to identify these policies. Policies before version 20 do not
support attributes at all and can be handled by all libsepol functions.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-03-15 21:52:03 +01:00
James Carter
dcd07fdcbf
libsepol/checkpolicy: Set user roles using role value instead of dominance
Roles in an optional block have two datums, one in the global block
and one in the avrule_decl where it is declared. The datum in the
global block does not have its dominace set. This is a problem because
the function set_user_role() sets the user's roles based on the global
datum's dominance ebitmap. If a user is declared with an associated role
that was declared in an optional block, then it will not have any roles
set for it because the dominance ebitmap is empty.

Example/
  # handle_unknown deny
  class CLASS1
  sid kernel
  class CLASS1 { PERM1 }
  type TYPE1;
  allow TYPE1 self:CLASS1 PERM1;
  role ROLE1;
  role ROLE1 types { TYPE1 };
  optional {
    require {
      class CLASS1 { PERM1 };
    }
    role ROLE1A;
    user USER1A roles ROLE1A;
  }
  user USER1 roles ROLE1;
  sid kernel USER1:ROLE1:TYPE1

In this example, USER1A would not have ROLE1A associated with it.

Instead of using dominance, which has been deprecated anyway, just
set the bit corresponding to the role's value in the user's roles
ebitmap in set_user_role().

Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

[N.I: added spaces around "-" operator]
2021-03-15 21:50:58 +01:00
James Carter
859857def9
libsepol: Remove unnecessary copying of declarations from link.c
At one point link_modules() might have needed this initial copying,
but now it serves no purpose, so remove it.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-03-15 21:48:16 +01:00
James Carter
6015b05d06
libsepol: Properly handle types associated to role attributes
Types associated to role attributes in optional blocks are not
associated with the roles that have that attribute. The problem
is that role_fix_callback is called before the avrule_decls are
walked.

Example/
  class CLASS1
  sid kernel
  class CLASS1 { PERM1 }
  type TYPE1;
  type TYPE1A;
  allow TYPE1 self : CLASS1 PERM1;
  attribute_role ROLE_ATTR1A;
  role ROLE1;
  role ROLE1A;
  roleattribute ROLE1A ROLE_ATTR1A;
  role ROLE1 types TYPE1;
  optional {
    require {
      class CLASS1 PERM1;
    }
    role ROLE_ATTR1A types TYPE1A;
  }
  user USER1 roles ROLE1;
  sid kernel USER1:ROLE1:TYPE1

In this example ROLE1A will not have TYPE1A associated to it.

Call role_fix_callback() after the avrule_decls are walked.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-03-15 21:48:07 +01:00
James Carter
f7431d0e0e
libsepol: Expand role attributes in constraint expressions
When creating the kernel binary policy, role attributes in constraint
expressions are not expanded. This causes the constraint expression
to refer to a non-existent role in the kernel policy. This can lead
to a segfault when converting the binary policy back to conf or CIL
source or when using policy tools such as seinfo.

Expand role attributes in constraint expressions when creating the
kernel binary policy.

Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
2021-03-15 21:47:56 +01:00
Petr Lautrbach
cf853c1a0c
Update VERSIONs to 3.2 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-03-04 16:42:59 +01:00
Nicolas Iooss
c672254329 restorecond: invalidate local_lock_fd properly when closing it
If flock(local_lock_fd,...) fails, in function local_server(), the file
descriptor to the lock file is closed but local_lock_fd is not reset to
-1. This leads to server() calling end_local_server(), which closes the
file descriptor again.

Fix this double-close issue by setting local_lock_fd to -1 after closing
it.

This issue was found by using Facebook's Infer static analyzer.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-03-03 15:53:32 +01:00
Nicolas Iooss
a9e0004f60
libsepol: invalidate the pointer to the policydb if policydb_init fails
Facebook's Infer static analyzer warns about a use-after-free issue in
libsemanage:

    int semanage_direct_mls_enabled(semanage_handle_t * sh)
    {
            sepol_policydb_t *p = NULL;
            int retval;

            retval = sepol_policydb_create(&p);
            if (retval < 0)
                    goto cleanup;

            /* ... */
    cleanup:
            sepol_policydb_free(p);
            return retval;
    }

When sepol_policydb_create() is called, p is allocated and
policydb_init() is called. If this second call fails, p is freed
andsepol_policydb_create() returns -1, but p still stores a pointer to
freed memory. This pointer is then freed again in the cleanup part of
semanage_direct_mls_enabled().

Fix this by setting p to NULL in sepol_policydb_create() after freeing
it.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-03-03 07:52:59 +01:00
lutianxiong
6238e02571
libsepol/cil: fix NULL pointer dereference in cil_fill_ipaddr
Found a NULL pointer dereference by fuzzing, reproducing:
    $ echo "(nodecon(())o(e()))" > tmp.cil
    $ secilc tmp.cil
    Segmentation fault (core dumped)

Add NULL check for addr_node->data in cil_fill_ipaddr.

Signed-off-by: lutianxiong <lutianxiong@huawei.com>
2021-02-27 21:39:18 +01:00
Petr Lautrbach
be065c4b44
sepolicy: Do not try to load policy on import
When a policy is inaccessible, scripts fail right "import sepolicy". With
this change we let the "sepolicy" module to import and move the policy
initialization before it's used for the first time.

Fixes:
    >>> import seobject
    Traceback (most recent call last):
      File "/usr/lib/python3.9/site-packages/sepolicy/__init__.py", line 171, in policy
        _pol = setools.SELinuxPolicy(policy_file)
      File "setools/policyrep/selinuxpolicy.pxi", line 73, in setools.policyrep.SELinuxPolicy.__cinit__
      File "setools/policyrep/selinuxpolicy.pxi", line 695, in setools.policyrep.SELinuxPolicy._load_policy
    PermissionError: [Errno 13] Permission denied: '//etc/selinux/targeted/policy/policy.33'

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.9/site-packages/seobject.py", line 33, in <module>
        import sepolicy
      File "/usr/lib/python3.9/site-packages/sepolicy/__init__.py", line 186, in <module>
        raise e
      File "/usr/lib/python3.9/site-packages/sepolicy/__init__.py", line 183, in <module>
        policy(policy_file)
      File "/usr/lib/python3.9/site-packages/sepolicy/__init__.py", line 173, in policy
        raise ValueError(_("Failed to read %s policy file") % policy_file)
    ValueError: Failed to read //etc/selinux/targeted/policy/policy.33 policy file

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-27 21:38:24 +01:00
Petr Lautrbach
d4d1f4ba7e
Update VERSIONs to 3.2-rc3 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-24 15:49:59 +01:00
Vit Mojzis
9b4e9c4bf0 gui: fix "file type" selection in fcontextPage
A change in seobject.py file_type_str_to_option made the "file type"
list not compatible with items in this ComboBox.
See commit 317743bbe2 ("python/semanage: fix export of fcontext socket
entries")

Avoid this in the future by populating the ComboBox using keys from
file_type_str_to_option.

This change disables translations on the file types, but those cause
other issues (adding file context fails the same way as with 'socket
file' since the translated strings differ form
file_type_str_to_option.keys, 'properties' of a file context entry
shows no file type for the same reason).

Fixes:
 Traceback (most recent call last):
  File "/usr/share/system-config-selinux/system-config-selinux.py", line 136, in add
    self.tabs[self.notebook.get_current_page()].addDialog()
  File "/usr/share/system-config-selinux/semanagePage.py", line 143, in addDialog
    if self.add() is False:
  File "/usr/share/system-config-selinux/fcontextPage.py", line 195, in add
    (rc, out) = getstatusoutput("semanage fcontext -a -t %s -r %s -f '%s' '%s'" % (type, mls, seobject.file_type_str_to_option[ftype], fspec))
 KeyError: 'socket file'

Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
2021-02-24 15:13:25 +01:00
Christian Göttsche
b69d77bcdb libsepol/cil: handle SID without assigned context when writing policy.conf
CIL permits not assigning a context to a SID, e.g. to an unused initial
SID, e.g. 'any_socket'.

When using the example policy from the SELinux Notebook,
https://github.com/SELinuxProject/selinux-notebook/blob/main/src/notebook-examples/cil-policy/cil-policy.cil,
secilc logs:

    No context assigned to SID any_socket, omitting from policy at cil-policy.cil:166

But secil2conf segfaults when writing the policy.conf:

    ../cil/src/cil_policy.c:274:2: runtime error: member access within null pointer of type 'struct cil_context'

Only print the sid context statement if a context was actually assigned.
The sid declaration is still included via cil_sid_decls_to_policy().

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-02-24 11:22:12 +01:00
bauen1
d464187c37 policycoreutils: sestatus belongs to bin not sbin
It is quite useful even to non-privileged users and doesn't require any
privileges to work, except for maybe -v.

Some tools hard code the old path, so a compatibility symlink is also
created.

Signed-off-by: Jonathan Hettwer <j2468h@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-24 11:22:03 +01:00
Petr Lautrbach
d59932a7ce policycoreutils: Resolve path in restorecon_xattr
Resolve pathname before selinux_restorecon_xattr() to prevent problems
with 'No Match' when relative path is used.

Fixes:
    # restorecon_xattr -v tmp
    ...
    tmp Digest: f9cd2da7141068bd2c08bc02fa471db63ac7d44c No Match

    # restorecon_xattr -v `pwd`/tmp
    ...
    /root/tmp Digest: f9cd2da7141068bd2c08bc02fa471db63ac7d44c Match

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-24 10:56:57 +01:00
Petr Lautrbach
142826a38e libselinux: fix segfault in add_xattr_entry()
When selabel_get_digests_all_partial_matches(), resp
get_digests_all_partial_matches() doesn't find a match,
calculated_digest is not initialized and followup memcmp() could
segfault. Given that calculated_digest and xattr_digest are already
compared in get_digests_all_partial_matches() and the function returns
true or false based on this comparison, it's not necessary to compare
these values again.

Fixes:
    # cd /root
    # mkdir tmp
    # restorecon -D -Rv tmp  # create security.sehash attribute
    # restorecon_xattr -d -v tmp
    specfiles SHA1 digest: afc752f47d489f3e82ac1da8fd247a2e1a6af5f8
    calculated using the following specfile(s):
    /etc/selinux/targeted/contexts/files/file_contexts.subs_dist
    /etc/selinux/targeted/contexts/files/file_contexts.subs
    /etc/selinux/targeted/contexts/files/file_contexts.bin
    /etc/selinux/targeted/contexts/files/file_contexts.homedirs.bin
    /etc/selinux/targeted/contexts/files/file_contexts.local.bin

    Segmentation fault (core dumped)

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-24 10:52:52 +01:00
James Carter
0861c659b5 libsepol: Validate policydb values when reading binary policy
Nicolas Iooss reports that fuzzing /usr/libexec/hll/pp with the
American Fuzzy Lop revealed that inconsistent policy modules could be
created that caused NULL dereferences and other problems.

When reading in a binary modular or kernel policy, check values in the
policydb to verify consistency. When reading in the data for commons,
classes, roles, types, users, booleans, sensitivities, and categories
verify that their value is between 1 and the number of primary
identifiers (value-1 is used to index the sym_val_to_name array for
all of these and the val_to_struct array for classes, roles, users,
and types.) Next all references in policy rules are checked to ensure
that they refer to a valid value.

It is possible for the type and role struct and name arrays to have
gaps in them. For roles, there will be gaps in the case of a kernel
policy created from a policy with role attributes, but nothing in the
policy will refer to any of the gaps. For types, there will be gaps
for any kernel policy with a version from 20 to 23, but, unfortunately,
there will be references to the gaps. This is because, while attributes
exist in these policies, they only exist in the type_attr_map. For
policies with versions between 20 and 23, it must be assumed that all
of the gaps and any references to them are valid. To check for
references to gaps, bitmaps are created to map where the gaps are and
all values are verified to be within the proper range and not within a
gap.

Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
2021-02-19 16:34:47 +01:00
James Carter
8f5409cf4c libsepol: Create function ebitmap_highest_set_bit()
Create the function ebitmap_highest_set_bit() which returns the position
of the highest bit set in the ebitmap.

The return value is valid only if the ebitmap is not empty. An empty
ebitmap will return 0.

Signed-off-by: James Carter <jwcart2@gmail.com>
2021-02-19 16:30:11 +01:00
Dominick Grift
49ff851c98 secilc: fixes cil_role_statements.md example
Signed-off-by: Dominick Grift <dominick.grift@defensec.nl>
Acked-by: James Carter <jwcart2@gmail.com>
2021-02-19 15:19:50 +01:00
Nicolas Iooss
398d2ceef9 libselinux: rename gettid() to something which never conflicts with the libc
Musl recently added a wrapper for gettid() syscall. There is no way to
detect this new version in a reliable way, so rename our gettid()
wrapper to a non-conflicting name.

Introduce a new function which, when using a libc known to provide a
wrapper for gettid(), calls it, and which, otherwise, performs the
syscall directly.

Anyway this function is only used on systems where /proc/thread-self
does not exist, which are therefore running Linux<3.17.

Fixes: https://github.com/SELinuxProject/selinux/issues/282
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-19 15:19:31 +01:00
Vit Mojzis
8f0f0a28aa selinux(8,5): Describe fcontext regular expressions
Describe which type of regular expression is used in file context
definitions and which flags are in effect.

Explain how local file context modifications are processed.

Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-19 15:18:05 +01:00
Christian Göttsche
5682c0d5f6 policycoreutils/fixfiles.8: add missing file systems and merge check and verify
Mention the supported file systems ext4, gfs2 and btrfs.

The options check and verify are interchangeable, merge their
description.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-19 15:15:37 +01:00
Christian Göttsche
9cc6b5cf40 libselinux/getconlist: report failures
Check the given context a priori, to print a more user friendly message,
opposed to a generic following get_ordered_context_list/_with_level
failure.

Notify the user about failures of get_ordered_context_list/_with_level,
so no-context-found and a failure results are distinguishable.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
2021-02-19 15:15:11 +01:00
James Carter
0451adebdf libsepol/cil: Destroy disabled optional blocks after pass is complete
Nicolas Iooss reports:
  I am continuing to investigate OSS-Fuzz crashes and the following one
  is quite complex. Here is a CIL policy which triggers a
  heap-use-after-free error in the CIL compiler:

  (class CLASS (PERM2))
  (classorder (CLASS))
  (classpermission CLSPRM)
  (optional o
      (mlsvalidatetrans x (domby l1 h1))
      (common CLSCOMMON (PERM1))
      (classcommon CLASS CLSCOMMON)
  )
  (classpermissionset CLSPRM (CLASS (PERM1)))

  The issue is that the mlsvalidatetrans fails to resolve in pass
  CIL_PASS_MISC3, which comes after the resolution of classcommon (in
  pass CIL_PASS_MISC2). So:

  * In pass CIL_PASS_MISC2, the optional block still exists, the
  classcommon is resolved and class CLASS is linked with common
  CLSCOMMON.
  * In pass CIL_PASS_MISC3, the optional block is destroyed, including
  the common CLSCOMMON.
  * When classpermissionset is resolved, function cil_resolve_classperms
  uses "common_symtab = &class->common->perms;", which has been freed.
  The use-after-free issue occurs in __cil_resolve_perms (in
  libsepol/cil/src/cil_resolve_ast.c):

    // common_symtab was freed
    rc = cil_symtab_get_datum(common_symtab, curr->data, &perm_datum);

The fundamental problem here is that when the optional block is
disabled it is immediately destroyed in the middle of the pass, so
the class has not been reset and still refers to the now destroyed
common when the classpermissionset is resolved later in the same pass.

Added a list, disabled_optionals, to struct cil_args_resolve which is
passed when resolving the tree. When optionals are disabled, they are
now added to this list and then are destroyed after the tree has been
reset between passes.

Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-17 18:27:39 +01:00
bauen1
038817036f secilc/docs: add custom color theme
Since the default pandoc themes either don't highlight everything or
don't fit the black/white color style of the html / pdf I've created my
own.

Signed-off-by: Jonathan Hettwer <j2468h@gmail.com>
2021-02-16 09:32:36 -05:00
bauen1
4c8d609498 secilc/docs: add syntax highlighting for secil
This is done by creating a somewhat rudimentary KDE syntax xml for
pandoc.

The default styles provided by pandoc don't look very good and don't
highlight e.g. the strings marked as builtin.

Signed-off-by: Jonathan Hettwer <j2468h@gmail.com>
2021-02-16 09:32:32 -05:00
bauen1
057d72af2d secilc/docs: use fenced code blocks for cil examples
Also fixes the occasional missing brackets as higlighted by my editor,
however the individual examples where not reviewed much closer.

secilc was chosen as language name because the compiler is named secilc
and outside of SELinux the name cil is less searchable and could lead to
confusion.

Signed-off-by: Jonathan Hettwer <j2468h@gmail.com>
2021-02-16 09:32:29 -05:00
Nicolas Iooss
32f8ed3d6b libsepol/cil: introduce intermediate cast to silence -Wvoid-pointer-to-enum-cast
clang 11.0.0 reports the following warning several times (when building
with "make CC=clang" from libsepol directory, in the default
configuration of the git repository):

    ../cil/src/cil_binary.c:1980:8: error: cast to smaller integer type
    'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
                    op = (enum cil_flavor)curr->data;
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Silence this warning by casting the pointer to an integer the cast to
enum cil_flavor.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-16 09:30:57 -05:00
Nicolas Iooss
4662bdc11c libsepol/cil: be more robust when encountering <src_info>
OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying
to compile the following policy:

    (<src_info>)

In cil_gen_src_info(), parse_current->next is NULL even though the code
expects that both parse_current->next and parse_current->next->next
exists.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28457
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-16 09:30:53 -05:00
Nicolas Iooss
6b56105858 libsepol/cil: fix NULL pointer dereference with empty macro argument
OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying
to compile the following policy:

    (macro m((name n))) (call m(()))

When calling the macro, the name (in variable "pc") is NULL, which
triggers a NULL pointer dereference when using it as a key in
__cil_insert_name(). The stack trace is:

    #0 0x7f4662655a85 in __strlen_avx2 (/usr/lib/libc.so.6+0x162a85)
    #1 0x556d0b6d150c in __interceptor_strlen.part.0 (/selinux/libsepol/fuzz/fuzz-secilc+0x44850c)
    #2 0x556d0ba74ed6 in symhash /selinux/libsepol/src/symtab.c:22:9
    #3 0x556d0b9ef50d in hashtab_search /selinux/libsepol/src/hashtab.c:186:11
    #4 0x556d0b928e1f in cil_symtab_get_datum /selinux/libsepol/src/../cil/src/cil_symtab.c:121:37
    #5 0x556d0b8f28f4 in __cil_insert_name /selinux/libsepol/src/../cil/src/cil_resolve_ast.c:96:2
    #6 0x556d0b908184 in cil_resolve_call1 /selinux/libsepol/src/../cil/src/cil_resolve_ast.c:2835:12
    #7 0x556d0b91b404 in __cil_resolve_ast_node /selinux/libsepol/src/../cil/src/cil_resolve_ast.c
    #8 0x556d0b91380f in __cil_resolve_ast_node_helper /selinux/libsepol/src/../cil/src/cil_resolve_ast.c:3773:7
    #9 0x556d0b932230 in cil_tree_walk_core /selinux/libsepol/src/../cil/src/cil_tree.c:263:9
    #10 0x556d0b932230 in cil_tree_walk /selinux/libsepol/src/../cil/src/cil_tree.c:307:7
    #11 0x556d0b932326 in cil_tree_walk_core /selinux/libsepol/src/../cil/src/cil_tree.c:275:9
    #12 0x556d0b932326 in cil_tree_walk /selinux/libsepol/src/../cil/src/cil_tree.c:307:7
    #13 0x556d0b911189 in cil_resolve_ast /selinux/libsepol/src/../cil/src/cil_resolve_ast.c:3941:8
    #14 0x556d0b798729 in cil_compile /selinux/libsepol/src/../cil/src/cil.c:550:7

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28544
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-16 09:30:49 -05:00
James Carter
0d0e47c7eb libsepol/cil: Fix integer overflow in the handling of hll line marks
Nicolas Iooss reports:
  OSS-Fuzz found an integer overflow when compiling the following
  (empty) CIL policy:

  ;;*lms 2147483647 a
  ; (empty line)

Change hll_lineno to uint32_t which is the type of the field hll_line
in struct cil_tree_node where the line number will be stored eventually.
Read the line number into an unsigned long variable using strtoul()
instead of strtol(). On systems where ULONG_MAX > UINT32_MAX, return
an error if the value is larger than UINT32_MAX.

Also change hll_expand to uint32_t, since its value will be either
0 or 1 and there is no need for it to be signed.

Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
2021-02-10 08:06:10 -05:00
Nicolas Iooss
1b36ace21b
libsepol: include header files in source files when matching declarations
It is good practise in C to include the header file that specifies the
prototype of functions which are defined in the source file. Otherwise,
the function prototypes which be different, which could cause unexpected
issues.

Add the include directives to do this.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-05 10:19:34 +01:00
Nicolas Iooss
1f1fa9d47e
libsepol: uniformize prototypes of sepol_mls_contains and sepol_mls_check
In libsepol/src/mls.c, functions sepol_mls_contains and sepol_mls_check
used "sepol_policydb_t * policydb" even though
libsepol/include/sepol/context.h used "const sepol_policydb_t *
policydb".

Add const qualifiers in mls.c in order to match the header file. Detect
such mismatching error at compile time by including the header file in
mls.c.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-05 10:19:34 +01:00
Nicolas Iooss
72a88d753d
libsepol: remove unused files
libsepol/src/roles.c contains functions which do not match its header
file libsepol/include/sepol/roles.h:

    // In roles.c
    int sepol_role_exists(sepol_handle_t * handle __attribute__ ((unused)),
                          sepol_policydb_t * p, const char *role, int *response)
    // In roles.h
    extern int sepol_role_exists(const sepol_policydb_t * policydb,
                                 const char *role, int *response);

and:

    // In roles.c
    int sepol_role_list(sepol_handle_t * handle,
                        sepol_policydb_t * p, char ***roles, unsigned int *nroles)
    // In roles.h
    extern int sepol_role_list(const sepol_policydb_t * policydb,
                               char ***roles, unsigned int *nroles);

Instead of fixing the parameter type (using sepol_handle_t or
sepol_policydb_t but not different ones), remove these functions, as
they appear not to be used. They are not exported in libsepol.so.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-05 10:19:30 +01:00
Petr Lautrbach
2c7c4a84c3
Update VERSIONs to 3.2-rc2 for release.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-03 11:26:28 +01:00
Nicolas Iooss
108c8edd95 scripts/release: make the script more robust, and release a source repository snapshot
Following Petr Lautrbach's suggestion, release a snapshot of the source
repository next to the individual archives which constitute a release.

While at it, make scripts/release more robust:

- Fix many warnings reported by shellcheck, by quoting strings.
- Use bash arrays for DIRS and DIRS_NEED_PREFIX
- Merge DIRS and DIRS_NEED_PREFIX into a single array, in order to
  produce SHA256 digests that are directly in alphabetical order, for
  https://github.com/SELinuxProject/selinux/wiki/Releases
- Use "set -e" in order to fail as soon as a command fails
- Change to the top-level directory at the start of the script, in order
  to be able to run it from anywhere.
- Use `cat $DIR/VERSION` and `git -C $DIR` instead of `cd $i ; cat VERSION`
  in order to prevent unexpected issues from directory change.

Finally, if version tags already exists, re-use them. This enables using
this script to re-generate the release archive (and check that they
really match the git repository). Currently, running scripts/release
will produce the same archives as the ones published in the 3.2-rc1
release (with the same SHA256 digests as the ones on the release page,
https://github.com/SELinuxProject/selinux/wiki/Releases). This helps to
ensure that the behaviour of the script is still fine.

Suggested-by: Petr Lautrbach <plautrba@redhat.com>
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-03 11:26:07 +01:00
James Carter
eba0ffee01 libsepol/cil: Fix heap-use-after-free when using optional blockinherit
This is based on a patch by Nicolas Iooss. He writes:
    When secilc compiles the following policy:

        (block b1
            (optional o1
                (blockinherit b1)
                (blockinherit x)
            )
        )

    it disables the optional block at pass 3 (CIL_PASS_BLKIN_LINK)
    because the block "x" does not exist.
    __cil_resolve_ast_last_child_helper() calls
    cil_tree_children_destroy() on the optional block, which destroys
    the two blockinherit statements. But the (blockinherit b1) node
    was referenced inside (block b1) node, in its block->bi_nodes list.
    Therefore, when this list is used at pass 4 (CIL_PASS_BLKIN_COPY),
    it contains a node which was freed: this triggers a use-after-free
    issue

    Fix this issue by removing blockinherit nodes from their lists of
    nodes block->bi_nodes when they are being destroyed. As
    cil_destroy_blockinherit() does not have a reference to the node
    containing the blockinherit data, implement this new logic in
    cil_tree_node_destroy().

    This issue was found while investigating a testcase from an OSS-Fuzz
    issue which seems unrelated (a Null-dereference READ in
    cil_symtab_get_datum,
    https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29861).

Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-03 10:44:01 +01:00
Nicolas Iooss
1048f8d329 libsepol/cil: unlink blockinherit->block link when destroying a block
The following CIL policy triggers a heap use-after-free in secilc
because when the blockinherit node is destroyed, the block node was
already destroyed:

    (block b2a)
    (blockinherit b2a)

Fix this by setting blockinherit->block to NULL when destroying block.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: James Carter <jwcart2@gmail.com>
2021-02-03 09:30:22 +01:00
Petr Lautrbach
57dd1f6520 policycoreutils/setfiles: Drop unused nerr variable
Suggested-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-03 09:29:28 +01:00
Nicolas Iooss
b320291888 libsepol/cil: fix memory leak when a constraint expression is too deep
When __cil_validate_constrain_expr() fails,
cil_constrain_to_policydb_helper() does not destroy the constraint
expression. This leads to a memory leak reported by OSS-Fuzz with the
following CIL policy:

    (class CLASS (PERM))
    (classorder (CLASS))
    (sid SID)
    (sidorder (SID))
    (user USER)
    (role ROLE)
    (type TYPE)
    (category CAT)
    (categoryorder (CAT))
    (sensitivity SENS)
    (sensitivityorder (SENS))
    (sensitivitycategory SENS (CAT))
    (allow TYPE self (CLASS (PERM)))
    (roletype ROLE TYPE)
    (userrole USER ROLE)
    (userlevel USER (SENS))
    (userrange USER ((SENS)(SENS (CAT))))
    (sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))

    (constrain
        (CLASS (PERM))
        (or
            (eq t1 TYPE)
            (or
                (eq t1 TYPE)
                (or
                    (eq t1 TYPE)
                    (or
                        (eq t1 TYPE)
                        (or
                            (eq t1 TYPE)
                            (eq t1 TYPE)
                        )
                    )
                )
            )
        )
    )

Add constraint_expr_destroy(sepol_expr) to destroy the expression.

Moreover constraint_expr_destroy() was not freeing all items of an
expression. Code in libsepol/src and checkpolicy contained while loop to
free all the items of a constraint expression, but not the one in
libsepol/cil. As freeing only the first item of an expression is
misleading, change the semantic of constraint_expr_destroy() to iterate
the list of constraint_expr_t and to free all items.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28938
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: James Carter <jwcart2@gmail.com>
2021-02-03 09:28:39 +01:00
Petr Lautrbach
c35919a703 libsemanage: sync filesystem with sandbox
Commit 331a109f91 ("libsemanage: fsync final files before rename")
added fsync() for policy files and improved situation when something
unexpected happens right after rename(). However the module store could
be affected as well. After the following steps module files could be 0
size:

1. Run `semanage fcontext -a -t var_t "/tmp/abc"`
2. Force shutdown the server during the command is run, or right after
   it's finished
3. Boot the system and look for empty files:
    # find /var/lib/selinux/targeted/ -type f -size 0 | wc -l
    1266

It looks like this situation can be avoided if the filesystem with the
sandbox is sync()ed before we start to rename() directories in the
store.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
2021-02-01 15:11:40 +01:00
Petr Lautrbach
be7f54cb1f setfiles: drop ABORT_ON_ERRORS and related code
`setfiles -d` doesn't have any impact on number of errors before it
aborts. It always aborts on first invalid context in spec file.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-01 15:02:42 +01:00
Petr Lautrbach
9207823c8f setfiles: Do not abort on labeling error
Commit 602347c742 ("policycoreutils: setfiles - Modify to use
selinux_restorecon") changed behavior of setfiles. Original
implementation skipped files which it couldn't set context to while the
new implementation aborts on them. setfiles should abort only if it
can't validate a context from spec_file.

Reproducer:

    # mkdir -p r/1 r/2 r/3
    # touch r/1/1 r/2/1
    # chattr +i r/2/1
    # touch r/3/1
    # setfiles -r r -v /etc/selinux/targeted/contexts/files/file_contexts r
    Relabeled r from unconfined_u:object_r:mnt_t:s0 to unconfined_u:object_r:root_t:s0
    Relabeled r/2 from unconfined_u:object_r:mnt_t:s0 to unconfined_u:object_r:default_t:s0
    setfiles: Could not set context for r/2/1:  Operation not permitted

r/3 and r/1 are not relabeled.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
2021-02-01 15:02:16 +01:00
Vit Mojzis
e12f71e82f python/sepolgen: allow any policy statement in if(n)def
"ifdef/ifndef" statements can be used to conditionally define
an interface, but this syntax is not recognised by sepolgen-ifgen.
Fix sepolgen-ifgen to allow any policy statement inside an
"ifdef/ifndef" statement.

Fixes:
        $ cat <<EOF > i.if
ifndef(`apache_manage_pid_files',`
        interface(`apache_manage_pid_files',`
                manage_files_pattern($1, httpd_var_run_t, httpd_var_run_t)
        ')
')

        #sepolgen-ifgen --interface=i.if
        i.if: Syntax error on line 2 interface [type=INTERFACE]
        i.if: Syntax error on line 4 ' [type=SQUOTE]

Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
[OM: s/fidef/ifdef/]
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
2021-01-27 17:02:45 +01:00
James Carter
f0d98f83d2
libsepol/cil: Fix heap-use-after-free in __class_reset_perm_values()
Nicolas Iooss reports:
  A few weeks ago, OSS-Fuzz got configured in order to fuzz the CIL
  policy compiler (cf.
  https://github.com/SELinuxProject/selinux/issues/215 and
  https://github.com/google/oss-fuzz/pull/4790). It reported a bunch of
  simple issues, for which I will submit patches. There are also more
  subtle bugs, like the one triggered by this CIL policy:

  (class CLASS (PERM))
  (classorder (CLASS))
  (sid SID)
  (sidorder (SID))
  (sensitivity SENS)
  (sensitivityorder (SENS))
  (type TYPE)
  (allow TYPE self (CLASS (PERM)))

  (block b
      (optional o
          (sensitivitycategory SENS (C)) ; Non-existing category
  triggers disabling the optional
          (common COMMON (PERM1))
          (classcommon CLASS COMMON)
          (allow TYPE self (CLASS (PERM1)))
      )
  )

  On my computer, secilc manages to build this policy fine, but when
  clang's Address Sanitizer is enabled, running secilc leads to the
  following report:

  $ make -C libsepol/src CC=clang CFLAGS='-g -fsanitize=address' libsepol.a
  $ clang -g -fsanitize=address secilc/secilc.c libsepol/src/libsepol.a
  -o my_secilc
  $ ./my_secilc -vv testcase.cil
  Parsing testcase.cil
  Building AST from Parse Tree
  Destroying Parse Tree
  Resolving AST
  Failed to resolve sensitivitycategory statement at testcase.cil:12
  Disabling optional 'o' at testcase.cil:11
  Resetting declarations
  =================================================================
  ==181743==ERROR: AddressSanitizer: heap-use-after-free on address
  0x6070000000c0 at pc 0x55ff7e445d24 bp 0x7ffe7eecfba0 sp
  0x7ffe7eecfb98
  READ of size 4 at 0x6070000000c0 thread T0
      #0 0x55ff7e445d23 in __class_reset_perm_values
  /git/selinux-userspace/libsepol/src/../cil/src/cil_reset_ast.c:17:17

The problem is that the optional branch is destroyed when it is disabled,
so the common has already been destroyed when the reset code tries to
access the number of common permissions, so that it can change the
value of the class permissions back to their original values.

The solution is to count the number of class permissions and then
calculate the number of common permissions.

Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
2021-01-20 16:53:39 +01:00