Python slip is not actively maintained anymore and it was used just as
a polkit proxy. It looks like polkit dbus interface is quite simple to
be used directly via python dbus module.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>From fclose(3):
Upon successful completion, 0 is returned. Otherwise, EOF is returned
and errno is set to indicate the error. In either case, any further
access (including another call to fclose()) to the stream results in
undefined behavior.
Fixes:
Error: USE_AFTER_FREE (CWE-672): [#def1]
libsemanage-3.2/src/direct_api.c:1023: freed_arg: "fclose" frees "fp".
libsemanage-3.2/src/direct_api.c:1034: use_closed_file: Calling "fclose" uses file handle "fp" after closing it.
# 1032|
# 1033| cleanup:
# 1034|-> if (fp != NULL) fclose(fp);
# 1035|
# 1036| return ret;
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
In case lstat(3) fails the memory is not free'd at the end of the for
loop, due to the control flow change by continue.
Found by scan-build.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
When specifying -o or -f more than once, the previous allocations leak.
Found by scan-build.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
- use multiple jobs
- define _FORTIFY_SOURCE=2 to enable checks on standard string handling
functions due to macro/intrinsic overloads or function attributes
- allow to override clang and scan-build binaries, i.e. for using
versioned ones
- set PYTHON_SETUP_ARGS accordingly on Debian
- enable common warning -Wextra
- print build result
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Using the '\0' character in strings in a CIL policy is not expected to
happen, and makes the flex tokenizer very slow. For example when
generating a file with:
python -c 'print("\"" + "\0"*100000 + "\"")' > policy.cil
secilc fails after 26 seconds, on my desktop computer. Increasing the
numbers of \0 makes this time increase significantly. But replacing \0
with another character makes secilc fail in only few milliseconds.
Fix this "possible denial of service" issue by forbidding \0 in strings
in CIL policies.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=36016
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
There are two problems that need to be addressed when resolving an
expression with category sets.
1. Only expand anonymous category sets in an expression.
Commit 982ec302b6 (libsepol/cil:
Account for anonymous category sets in an expression) attempted to
properly handle anonymous category sets when resolving category
expressions. Unfortunately, it did not check whether a category set
was actually an anonymous category set and expanded all category
sets in an expression. If a category set refers to itself in the
expression, then everything from the name of the category set to the
end of the expression is ignored.
For example, the rule "(categoryset cs (c0 cs c1 c2))", would be
equivalent to the rule "(categoryset cs (c0))" as everything from
"cs" to the end would be dropped. The secilc-fuzzer found that the
rule "(categoryset cat (not cat))" would cause a segfault since
"(not)" is not a valid expression and it is assumed to be valid
during later evaluation because syntax checking has already been
done.
Instead, check whether or not the category set is anonymous before
expanding it when resolving an expression.
2. Category sets cannot be used in a category range
A category range can be used to specify a large number of categories.
The range "(range c0 c1023)" refers to 1024 categories. Only categories
and category aliases can be used in a range. Determining if an
identifier is a category, an alias, or a set can only be done after
resolving the identifer.
Keep track of the current operator as an expression is being resolved
and if the expression involves categories and a category set is
encountered, then return an error if the expression is a category
range.
Signed-off-by: James Carter <jwcart2@gmail.com>
Make it more obvious which parameters are read-only and not being
modified and allow callers to pass const pointers.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
checkpolicy.c: In function ‘main’:
checkpolicy.c:1000:25: error: ‘tsid’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
1000 | printf("if_sid %d default_msg_sid %d\n", ssid, tsid);
| ^
checkpolicy.c: In function ‘main’:
checkpolicy.c:971:25: error: ‘tsid’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
971 | printf("fs_sid %d default_file_sid %d\n", ssid, tsid);
| ^
Found by GCC 11 with LTO enabled.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
policy_define.c: In function ‘define_te_avtab_extended_perms’:
policy_define.c:1946:17: error: potential null pointer dereference [-Werror=null-dereference]
1946 | r->omit = omit;
| ^
In the case of `r` being NULL, avrule_read_ioctls() would return
with its parameter `rangehead` being a pointer to NULL, which is
considered a failure in its caller `avrule_ioctl_ranges`.
So it is not necessary to alter the return value.
Found by GCC 11 with LTO enabled.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `cladatum` is otherwise always assigned before used, so
these two assignments without a follow up usages are not needed.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Follow the project style of no declaration after statement.
Found by the GCC warning -Wdeclaration-after-statement.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
test/dispol.c:288:4: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(buf, sizeof(buf), "unknown (%d)", i);
^
test/dismod.c:830:4: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(buf, sizeof(buf), "unknown (%d)", i);
^
Found by Cppcheck.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `id` is guaranteed to be non-NULL due to the preceding
while condition.
policy_define.c:1171:7: style: Condition '!id' is always false [knownConditionTrueFalse]
if (!id) {
^
policy_define.c:1170:13: note: Assuming that condition 'id=queue_remove(id_queue)' is not redundant
while ((id = queue_remove(id_queue))) {
^
policy_define.c:1171:7: note: Condition '!id' is always false
if (!id) {
^
Found by Cppcheck.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
checkpolicy.c:504:20: style: The statement 'if (policyvers!=n) policyvers=n' is logically equivalent to 'policyvers=n'. [duplicateConditionalAssign]
if (policyvers != n)
^
checkpolicy.c:505:17: note: Assignment 'policyvers=n'
policyvers = n;
^
checkpolicy.c:504:20: note: Condition 'policyvers!=n' is redundant
if (policyvers != n)
^
Found by Cppcheck
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The compiler option -pipe does not affect the generated code; it affects
whether the compiler uses temporary files or pipes. As the benefit might
vary from system to system usually its up to the packager or build
framework to set it.
Also these are the only places where the flag is used.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Pass CFLAGS when invoking CC at link time, it might contain optimization
or sanitizer flags required for linking.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Clang complains:
ibendport_record.c: In function ‘sepol_ibendport_get_ibdev_name’:
ibendport_record.c:169:2: error: ‘strncpy’ specified bound 64 equals destination size [-Werror=stringop-truncation]
169 | strncpy(tmp_ibdev_name, ibendport->ibdev_name, IB_DEVICE_NAME_MAX);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ibendport_record.c: In function ‘sepol_ibendport_set_ibdev_name’:
ibendport_record.c:189:2: error: ‘strncpy’ specified bound 64 equals destination size [-Werror=stringop-truncation]
189 | strncpy(tmp, ibdev_name, IB_DEVICE_NAME_MAX);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
strncpy(3) does not NUL-terminate the destination if the source is of
the same length or longer then the specified size.
The source of these copies are retrieved from
sepol_ibendport_alloc_ibdev_name(), which allocates a fixed amount of
IB_DEVICE_NAME_MAX bytes.
Reduce the size to copy by 1 of all memory regions allocated by
sepol_ibendport_alloc_ibdev_name().
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Avoid implicit conversions from signed to unsigned values, found by
UB sanitizers, by using unsigned values in the first place.
expand.c:1644:18: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
expand.c:2892:24: runtime error: implicit conversion from type 'int' of value -2 (32-bit, signed) to type 'unsigned int' changed the value to 4294967294 (32-bit, unsigned)
policy_define.c:2344:4: runtime error: implicit conversion from type 'int' of value -1048577 (32-bit, signed) to type 'unsigned int' changed the value to 4293918719 (32-bit, unsigned)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Unsigned integer overflow is well-defined and not undefined behavior.
But it is still useful to enable undefined behavior sanitizer checks on
unsigned arithmetic to detect possible issues on counters or variables
with similar purpose.
Annotate functions, in which unsigned overflows are expected to happen,
with the respective Clang function attribute[1].
GCC does not support sanitizing unsigned integer arithmetic[2].
avtab.c:76:2: runtime error: unsigned integer overflow: 6 * 3432918353 cannot be represented in type 'unsigned int'
policydb.c:795:42: runtime error: unsigned integer overflow: 8160943042179512010 * 11 cannot be represented in type 'unsigned long'
symtab.c:25:12: runtime error: left shift of 1766601759 by 4 places cannot be represented in type 'unsigned int'
[1]: https://clang.llvm.org/docs/AttributeReference.html#no-sanitize
[2]: https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Unsigned integer overflow is well-defined and not undefined behavior.
It is commonly used for hashing or pseudo random number generation.
But it is still useful to enable undefined behavior sanitizer checks on
unsigned arithmetic to detect possible issues on counters or variables
with similar purpose or missed overflow checks on user input.
Use a spaceship operator like comparison instead of subtraction.
policydb.c:851:24: runtime error: unsigned integer overflow: 801 - 929 cannot be represented in type 'unsigned int'
Follow-up of: 1537ea8412 ("libsepol: avoid unsigned integer overflow")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
commits 37863b0b14 (libsepol/cil:
Improve degenerate inheritance check) and
74d00a8dec (libsepol/cil: Detect
degenerate inheritance and exit with an error) attempted to detect
and exit with an error when compiling policies that have degenerate
inheritances. These policies result in the exponential growth of memory
usage while copying the blocks that are inherited.
There were two problems with the previous attempts to detect this
bad inheritance problem. The first is that the quick check using
cil_possible_degenerate_inheritance() did not detect all patterns
of degenerate inheritance. The second problem is that the detection
of inheritance loops during the CIL_PASS_BLKIN_LINK pass did not
detect all inheritance loops which made it possible for the full
degenerate inheritance checking done with
cil_check_for_degenerate_inheritance() to have a stack overflow
when encountering the inheritance loops. Both the degenerate and
loop inheritance checks need to be done at the same time and done
after the CIL_PASS_BLKIN_LINK pass. Otherwise, if loops are being
detected first, then a degenerate policy can cause the consumption
of all system memory and if degenerate policy is being detected
first, then an inheritance loop can cause a stack overflow.
With the new approach, the quick check is eliminated and the full
check is always done after the CIL_PASS_BLKIN_LINK pass. Because
of this the "inheritance_check" field in struct cil_resolve_args
is not needed and removed and the functions
cil_print_recursive_blockinherit(), cil_check_recursive_blockinherit(),
and cil_possible_degenerate_inheritance() have been deleted. The
function cil_count_potential() is renamed cil_check_inheritances()
and has checks for both degenerate inheritance and inheritance loops.
The inheritance checking is improved and uses an approach similar
to commit c28525a26f (libsepol/cil:
Properly check for loops in sets).
As has been the case with these degenerate inheritance patches,
these issues were discovered by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
mcstransd.c:72:35: error: empty expression statement has no effect;
remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
log_debug("%s\n", "cleanup_exit");
^
Replace the empty log_debug substitution with a do { ... } while (0)
construction to silence this warning.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
secon.c:686:3: error: empty expression statement has no effect;
remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
};
^
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
checkpolicy.c:740:33: error: empty expression statement has no
effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
FGETS(ans, sizeof(ans), stdin);
^
Introduce "do { } while (0)" blocks to silence such warnings.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
genhomedircon.c:742:67: error: empty expression statement has no
effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
const semanage_seuser_t **u2 = (const semanage_seuser_t **) arg2;;
^
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
sha1.c:90:21: error: empty expression statement has no effect;
remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
R0(a,b,c,d,e, 0); R0(e,a,b,c,d, 1); R0(d,e,a,b,c, 2); R0(c,d,e,a,b, 3);
^
In file included from selinux_restorecon.c:39:
./label_file.h:458:15: error: empty expression statement has no
effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
lineno);
^
Introduce "do { } while (0)" blocks to silence such warnings.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
../cil/src/cil_binary.c:4293:22: error: empty expression statement
has no effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
mix(k->target_class);
^
../cil/src/cil_binary.c:4294:21: error: empty expression statement
has no effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
mix(k->target_type);
^
../cil/src/cil_binary.c:4295:21: error: empty expression statement
has no effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
mix(k->source_type);
^
../cil/src/cil_binary.c:4296:19: error: empty expression statement
has no effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
mix(k->specified);
^
Use a do { ... } while (0) construction to silence this warning.
Moreover the same warning appears when using two semicolons to end a
statement. Remove such occurrences, like what was already done in commit
811185648a ("libsepol: drop repeated semicolons").
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When __cil_verify_map_class() verifies a classpermission, it calls
__verify_map_perm_classperms() on each item. If the first item reports a
failure and the next one succeeds, the failure is overwritten in
map_args->rc. This is a bug which causes a NULL pointer dereference in
the CIL compiler when compiling the following policy:
(sid SID)
(sidorder (SID))
(class CLASS (PERM1))
(classorder (CLASS))
(classpermission CLSPERM)
(classpermissionset CLSPERM (CLASS (PERM1)))
(classmap files (CLAMAPxx x))
(classmapping files CLAMAPxx CLSPERM)
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=30286
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Fix the following build failure with gcc 4.8 which is raised since
version 3.2 and
156dd0de5c
getseuser.c:53:2: error: 'for' loop initial declarations are only allowed in C99 mode
for (int i = 0; i < n; i++)
^
Fixes:
- http://autobuild.buildroot.org/results/37eb0952a763256fbf6ef3c668f6c95fbdf2dd35
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Provide the option "-Q" or "--qualified-names" to indicate that the
policy is using qualified names.
Using qualified names means that declaration names can have "dots"
in them, but blocks, blockinherits, blockabstracts, and in-statements
are not allowed in the policy.
The libsepol function cil_set_qualified_names() is called with the
desired value for the CIL db's "qualified_names" field.
Signed-off-by: James Carter <jwcart2@gmail.com>
Provide the option "-Q" or "--qualified-names" to indicate that the
policy is using qualified names.
Using qualified names means that declaration names can have "dots"
in them, but blocks, blockinherits, blockabstracts, and in-statements
are not allowed in the policy.
The libsepol function cil_set_qualified_names() is called with the
desired value for the CIL db's "qualified_names" field.
Signed-off-by: James Carter <jwcart2@gmail.com>
Provide the option "-Q" or "--qualified-names" to indicate that the
policy is using qualified names.
Using qualified names means that declaration names can have "dots"
in them, but blocks, blockinherits, blockabstracts, and in-statements
are not allowed in the policy.
The libsepol function cil_set_qualified_names() is called with the
desired value for the CIL db's "qualified_names" field.
Signed-off-by: James Carter <jwcart2@gmail.com>
Qualified names have "dots" in them. They are generated when a CIL
policy is compiled and come from declarations in blocks. If a kernel
policy is decompiled into a CIL policy, the resulting policy could
have declarations that use qualified names. Compiling this policy would
result in an error because "dots" in declarations are not allowed.
Qualified names in a policy are normally used to refer to the name of
identifiers, blocks, macros, or optionals that are declared in a
different block (that is not a parent). Name resolution is based on
splitting a name based on the "dots", searching the parents up to the
global namespace for the first block using the first part of the name,
using the second part of the name to lookup the next block using the
first block's symbol tables, looking up the third block in the second's
symbol tables, and so on.
To allow the option of using qualified names in declarations:
1) Create a field in the struct cil_db called "qualified_names" which
is set to CIL_TRUE when qualified names are to be used. This field is
checked in cil_verify_name() and "dots" are allowed if qualified names
are being allowed.
2) Only allow the direct lookup of the whole name in the global symbol
table. This means that blocks, blockinherits, blockabstracts, and in-
statements cannot be allowed. Use the "qualified_names" field of the
cil_db to know when using one of these should result in an error.
3) Create the function cil_set_qualified_names() that is used to set
the "qualified_names" field. Export the function in libsepol.
Signed-off-by: James Carter <jwcart2@gmail.com>
When disabling optionals, the AST needs to be reset only if one
of the optional blocks being disabled contains a declaration.
Call the function cil_tree_subtree_has_decl() for each optional
block being disabled and only reset the AST if one of them has
a declaration in it.
Signed-off-by: James Carter <jwcart2@gmail.com>
Create the function cil_tree_subtree_has_decl() that returns CIL_TRUE
if the subtree has a declaration in it and CIL_FALSE otherwise.
Signed-off-by: James Carter <jwcart2@gmail.com>
The commit 74d00a8dec (libsepol/cil:
Detect degenerate inheritance and exit with an error) detects the
use of inheritance (mostly by the secilc-fuzzer and not in any real
policies) that results in the exponential growth of the policy through
the copying of blocks that takes place with inheritance in CIL.
Unfortunately, the check takes place during the pass when all the
blocks are being copied, so it is possible to consume all of a system's
memory before an error is produced.
The new check happens in two parts. First, a check is made while the
block inheritance is being linked to the block it will inherit. In
this check, all of the parent nodes of the inheritance rule up to the
root node are checked and if enough of these blocks are being inherited
(>= CIL_DEGENERATE_INHERITANCE_DEPTH), then a flag is set for a more
in-depth check after the pass. This in-depth check will determine the
number of potential inheritances that will occur when resolving the
all of the inheritance rules. If this value is greater than
CIL_DEGENERATE_INHERITANCE_GROWTH * the original number of inheritance
rules and greater than CIL_DEGENERATE_INHERITANCE_MINIMUM (which is
set to 0x1 << CIL_DEGENERATE_INHERITANCE_DEPTH), then degenerate
inheritance is determined to have occurred and an error result will
be returned.
Since the potential number of inheritances can quickly be an extremely
large number, the count of potential inheritances is aborted as soon
as the threshold for degenerate inheritance has been exceeded.
Normal policies should rarely, if ever, have the in-depth check occur.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
It is possible to create bad behaving policy that can consume all
of a system's memory (one way is through the use of inheritance).
Analyzing these policies shows that most of the memory usage is for
the block symtabs.
Most of the nineteen symtabs will most likely never be used, so give
these symtabs an initial size of 1. The others are given more
appropriate sizes.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When marking a type attribute as used in a neverallow (to help determine
whether or not it should be expanded), check if the attribute's expression
list is empty (no attributes are associated with it) before iterating
over the list.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When "glblub" was added as a default for the defaultrange rule, the
syntax array was updated because the "glblub" default does not need
to specify a range of "low", "high", or "low-high". Unfortunately,
additional checking was not added for the "source" and "target"
defaults to make sure they specified a range. This means that using
the "source" or "target" defaults without specifying the range will
result in a segfault.
When the "source" or "target" defaults are used, check that the rule
specifies a range as well.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
