The function __cil_verify_rule() is currently not used as all call sites
are commented out. Keep the function for future references.
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
checkpolicy(8) since 01b88ac3 ("checkpolicy: warn on bogus IP address or
netmask in nodecon statement") warns about host bits set in IPv6
addresses.
Adjust IPv6 netmasks in the libsepol tests so that the used address ::1
does not set any host bits and running the tests does not print several
of the following warnings:
net_contexts:15:WARNING 'host bits in ipv6 address set' at token '' on line 594:
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Since test-linker-cond-map.h and test-linker-types.h references
policydb_t, include the policydb header file.
Signed-off-by: James Carter <jwcart2@gmail.com>
`hashtab_search()` takes a const_hashtab_key_t, alias `const char*` as
second key parameter type. Do not unnecessarily cast variables of type
`const char*` to hashtab_key_t, alias `char*`.
policydb.c: In function ‘policydb_string_to_security_class’:
policydb.c:4164:39: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
4164 | (hashtab_key_t) class_name);
| ^
policydb.c: In function ‘policydb_string_to_av_perm’:
policydb.c:4184:25: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
4184 | (hashtab_key_t)perm_name);
| ^
policydb.c:4193:25: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
4193 | (hashtab_key_t)perm_name);
| ^
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check the actual pointer which memory was assigned to, not its parent
array pointer.
services.c:810:14: warning: Assigned value is garbage or undefined [core.uninitialized.Assign]
**r_buf = **new_buf;
^ ~~~~~~~~~
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Since calloc() will return an error if nmemb * size would overflow,
just use it instead of mallocarray(). This also allows code that
initializes the array to zero to be removed.
Signed-off-by: James Carter <jwcart2@gmail.com>
Export functions needed for converting security identifiers from and to
strings and functions computing security server decisions. These can
be used to debug or run tests on binary policies without running on a
SELinux enabled kernel.
TODO:
These functions have currently a non consistent return behavior: some
are returning -1 on failure and set errno most but not all of the time,
some return a negative errno like value.
Maybe this should be addressed before exporting them?
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The typedef `sepol_security_context_t` is used for contexts. For the
read-only input parameter in `sepol_context_to_sid()`
`const sepol_security_context_t` is used as type, which does not expand
to the expected `const char*` but `char *const`.
Introduce a corresponding typedef for `const char*`.
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Add a wrapper around the utility function sepol_av_to_string() on the
service internal policy. This allows callers to convert a permission
bit set into a string representation without access to the internal
policy structure.
Acked-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
It seems to be unused since its initial addition in 76ba6eaa
("Squashed 'libsepol/cil/' changes from 08520e9..28ad56e").
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-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>
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>
Acked-by: James Carter <jwcart2@gmail.com>
Check return values of memory allocation functions and propagate their
failure.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
GCC 12 produces an array-bounds warning:
In file included from ../include/sepol/policydb/context.h:23,
from ../include/sepol/policydb/policydb.h:62,
from ../cil/src/cil_binary.c:41:
In function ‘mls_level_init’,
inlined from ‘mls_level_destroy’ at ../include/sepol/policydb/mls_types.h:99:2,
inlined from ‘mls_level_destroy’ at ../include/sepol/policydb/mls_types.h:92:20,
inlined from ‘mls_range_destroy’ at ../include/sepol/policydb/mls_types.h:149:2,
inlined from ‘cil_rangetransition_to_policydb’ at ../cil/src/cil_binary.c:3231:6:
../include/sepol/policydb/mls_types.h:89:9: error: ‘memset’ offset [0, 23] is out of the bounds [0, 0] [-Werror=array-bounds]
89 | memset(level, 0, sizeof(mls_level_t));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/sepol/policydb/mls_types.h:89:9: error: ‘memset’ offset [0, 23] is out of the bounds [0, 0] [-Werror=array-bounds]
cc1: all warnings being treated as errors
This is a false positive, by inspecting the code and compiling with -O3
and -flto.
Closes: https://github.com/SELinuxProject/selinux/issues/339
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
When validating a policydb, validate the conditional expressions
including the values of the booleans within them.
Found by oss-fuzz (#45523)
Signed-off-by: James Carter <jwcart2@gmail.com>
Use calloc() instead of mallocarray() so that everything is
initialized to zero to prevent the use of unitialized memory when
validating malformed binary policies.
Found by oss-fuzz (#45493)
Signed-off-by: James Carter <jwcart2@gmail.com>
The MAX_LOG_SIZE is 512. It is possible that a log message could
exceed the max size (such as for neverallowx rules). If so, then
write out "<LOG MESSAGE TRUNCATED>", so that it is obvious that
the log message has been truncated.
Reported-by: Jonathan Hettwer <j2468h@googlemail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
When writing a conf file or CIL policy out from a kernel binary,
do not write out a constraint rule if it has no permissions.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since CIL allows permission expressions, it is possible for the
expression to evaluate to no permissions. If this is the case,
then don't add the constraint.
Signed-off-by: James Carter <jwcart2@gmail.com>
Shorten "CAPABILITY" to "CAP" following the kernel naming convention.
The SELinux policy capability enum names should now follow the
"POLICYDB_CAP_XXX" format.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
Acked-by: James Carter <jwcart2@gmail.com>
Extended permission and neverallow rules are not permitted in
conditional statements.
This causes issues on policy optimization where avtab_search() might
return a non extended permission rule when searching for one.
Found by oss-fuzz (#45327)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
When validating a policydb, do a more thorough validation of the
constraints.
- No permissions if it is a (mls)validatetrans.
- Only mlsvalidatetrans can use u3, r3, and t3.
- Expressions not involving types should have an empty type set.
- Only "==" and "!=" are allowed when there are names.
- If names are not used in an expression then both the names bitmap
and the type set should be empty.
- Only roles and mls expressions can used "dom", "domby", and "incomp".
- An mls expression cannot use names.
- If the expression is "not", "and", or "or", then the names bitmap
and the type set should be empty.
Signed-off-by: James Carter <jwcart2@gmail.com>
In f0a5f6e, calls to reallocarray were introduced. Ensure that the
correct header (private.h) is included when necessary.
Fixes: f0a5f6e ("libsepol: use reallocarray wrapper to avoid overflows")
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
Acked-by: James Carter <jwcart2@gmail.com>
Test: Built using Android CI (glibc 2.17)
On the first loop iteration the variables `r_buf` and `reason_buf_used`
are NULL respective 0. Please UBSAN by not adding them but instead
directly assign NULL.
services.c:800:16: runtime error: applying zero offset to null pointer
#0 0x4d4fce in constraint_expr_eval_reason ./libsepol/src/services.c:800:16
#1 0x4cf31a in sepol_validate_transition_reason_buffer ./libsepol/src/services.c:1079:8
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
When an assertion fails, the error message refers to a generic
"policy.conf" file. When parsing a policy in checkpolicy, populate its
name using the original filename (source_filename is still build using
the #line directives within the policy).
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
[Merge conflicts fixed by: James Carter <jwcart2@gmail.com>]
Signed-off-by: James Carter <jwcart2@gmail.com>
When there are conflicting context rules, the location of the
conflicting rules are written out. If there are many duplicates of
the same context rule, there will be many pairs of conflicts written
out. This hides the fact that all of the rules are the same and can
make it hard to see the different conflicts.
First, since these are warnings and not reported at the default log
verbosity level (which only reports errors), only search for the
locations of the conflicting rules when the verbosity level means
that the warnings will actually be reported.
Second, Report all the duplicate conflicting rules together.
Third, Report the first four conflicts of the same rule if when
the verbosity level is at CIL_WARN ("-v") and report all of them
when the verbosity level is at CIL_INFO or higher ("-v -v").
Fixes problem found by oss-fuzz (#39735)
Signed-off-by: James Carter <jwcart2@gmail.com>
When there is a neverallow violation, a search is made for all of
the rules that violate the neverallow. The violating rules as well
as their parents are written out to make it easier to find these
rules.
If there is a lot of rules that violate a neverallow, then this
amount of reporting is too much. Instead, only print out the first
four rules (with their parents) that match the violated neverallow
rule along with the total number of rules that violate the
neverallow at the default log level. Report all the violations when
at a higher verbosity level.
Signed-off-by: James Carter <jwcart2@gmail.com>
Commit 4b2e2a248e (libsepol/cil: Limit
the amount of reporting for bounds failures) limited the number of
bounds failures that were reported to the first two matching rules
for the first two bad rules.
Instead, report the first two matching rules for the first four bad
rules at the default log level and report all matching rules for all
bad rules for higher verbosity levels.
Signed-off-by: James Carter <jwcart2@gmail.com>
Not all violations of neverallowxperm rules were being reported.
In check_assertion_extended_permissions_avtab(), a break was
performed after finding a match rather than just returning right
away. This means that if other src and tgt pairs were checked
afterward that did not match, then no match would be reported.
Example:
allow attr attr:CLASS ioctl;
allowxperm attr attr:CLASS ioctl 0x9401;
allowxperm t1 self:CLASS ioctl 0x9421;
neverallowxperm attr self:CLASS ioctl 0x9421;
Would result in no assertion violations being found.
Another problem was that the reporting function did not properly
recognize when there was a valid allowxperm rule and falsely
reported additional violations that did not exist. (There had
to be at least one legitimate violation.)
Using the same example as above (and assuming t1 and t2 both have
attribute attr), the following would be reported as:
neverallowxperm on line 4 of policy.conf (or line 4 of policy.conf)
violated by
allowxperm t1 t1:CLASS ioctl { 0x9421 };
neverallowxperm on line 4 of policy.conf (or line 4 of policy.conf)
violated by
allow t2 t2:CLASS4 { ioctl };
There is no violation for t2 because there is a valid allowxperm
rule for it.
With this patch, only the first error message (which is the correct
one) is printed.
Signed-off-by: James Carter <jwcart2@gmail.com>
The changes are the same as in a patch sent by Christian Göttsche
<cgzones@googlemail.com> to support adding not-self to neverallowxperm
checking, but it is needed for normal neverallowxperm checking as well
and the following explanation reflects that.
When reporting neverallowxperm violations, the avtab is searched to
find the rule that violates the assertion. If the avtab pointer of
the args is not set, then it will report the error as if no extended
permissions existed for the source and target (so allowing the ioctl
permission at all violates the neverallowxperm).
Example (where t1 has attribute attr):
allow attr attr:CLASS ioctl;
allowxperm attr attr:CLASS ioctl 0x9411;
neverallowxperm t1 self:CLASS ioctl 0x9411;
Would be reported as:
neverallowxperm on line 3 of policy.conf (or line 3 of policy.conf)
violated by
allow t1 t1:CLASS { ioctl };
Instead of:
neverallowxperm on line 3 of policy.conf (or line 3 of policy.conf)
violated by
allowxperm attr attr:CLASS ioctl { 0x9411 };
Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
When checking for violations of neverallow rules, if the neverallow
uses self, then the src and tgt must be the same when checking
extended permissions and when reporting violations.
Example:
allow attr attr : CLASS PERM;
neverallow attr self : CLASS PERM;
If the types t1 and t2 have attribute attr, then the violations
that would be reported would be:
allow t1 t1 : CLASS PERM;
allow t1 t2 : CLASS PERM;
allow t2 t1 : CLASS PERM;
allow t2 t2 : CLASS PERM;
instead of:
allow t1 t1 : CLASS PERM;
allow t2 t2 : CLASS PERM;
Signed-off-by: James Carter <jwcart2@gmail.com>
The value returned from report_assertion_extended_permissions() is
the nubmer of errors, so call it that instead of ret.
Signed-off-by: James Carter <jwcart2@gmail.com>
In both check_assertion_extended_permissions() and
report_assertion_avtab_matches(), when checking for a match involving
a rule using self, the matches between the source and target of the
rule being checked are found using ebitmap_and() and then the matches
between that result and the source of the neverallow are found using
another ebitmap_and() call.
Since the matches between the sources of the rule being checked and
the neverallow have already been found, just find the matches between
that result and the target of the rule being checked. This only
requires one call to ebitmap_and() instead of two.
Signed-off-by: James Carter <jwcart2@gmail.com>
When check_assertion_extended_permissions() is called, it has already
been determined that there is a match, and, since neither the class
nor the permissions are used, there is no need for the check.
Signed-off-by: James Carter <jwcart2@gmail.com>
Inorder to differentiate errors from matches, use "(rc < 0)" when
calling ebitmap_* functions while checking neverallow rules.
Also, just use rc instead of having a separate variable (ret) in
check_assertion_extended_permissions().
Signed-off-by: James Carter <jwcart2@gmail.com>
If a neverallow has target types as well as using self and a match
is found with the target types, then self does not even need to
be checked, since the rule is already in violation of the assertion.
So move the check for a match of the target types before dealing with
self.
Signed-off-by: James Carter <jwcart2@gmail.com>
In check_assertion_avtab_match(), for the functions that do not return
an error, but only returns 0 or 1 depending on if a match is found,
call the function in an if statement.
Signed-off-by: James Carter <jwcart2@gmail.com>
Return an error if check_assertion_extended_permissions() returns
an error instead of treating it as an assertion violation.
Signed-off-by: James Carter <jwcart2@gmail.com>
An out of memory condition is unlikely and the general message
that an error occured while checking neverallows is sufficient.
Signed-off-by: James Carter <jwcart2@gmail.com>
Instead of calling report_assertion_failures() and treating an
error like it was a neverallow violation, just return an error.
Signed-off-by: James Carter <jwcart2@gmail.com>
Check the type for type and role sets is valid.
Check the scope of a scope datum is valid.
Check the flavor and flags of a type datum are valid.
Check xperms are set if and only if it is an extended permission avrule.
Check xperms has a valid specified field.
Check the flag of avrule blocks is valid.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check the permission bitset in normal constraints is not empty and has
no invalid bits set.
Check the names and type_names members are empty in case they are not
used.
Check the operator and attribute type are not set for simple expression
types.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The callback function apply in hashtap_map has a return type of int and
can return -1 on error. Use int as type to save the return value to
avoid implicit conversions:
hashtab.c:236:10: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Map classes use the same struct as kernel classes, but only the kernel
class uses the pointer to a common class. When resolving a classcommon,
make sure that the class that is found is a kernel class and not a
map class. If not, then return an error.
Found by oss-fuzz (#43209)
Signed-off-by: James Carter <jwcart2@gmail.com>
Since abstract blocks will not appear in the final policy, do not
resolve names to a declaration inside one.
When resolving blockabstract rules, they must be collected in a list
and processed at the end of the pass because if a parent block is
marked as abstract, then a blockabstract rule for a sub-block will
fail to resolve.
Found by oss-fuzz (#42981)
Signed-off-by: James Carter <jwcart2@gmail.com>
If a block is marked as abstract, then it will be skipped during
every pass after blockabstracts are resolved (only tunables,
in-befores, and blockinherits are before blockabstracts), so mark
all of its sub-blocks as abstract to reflect their actual status.
Signed-off-by: James Carter <jwcart2@gmail.com>
Do not copy any blockabstract statements when copying a block to
resolve a blockinherit statement. Inheriting a block from what was
just inherited does not work, so there is no reason to create an
abstract block.
Signed-off-by: James Carter <jwcart2@gmail.com>
When converting an ebitmap into a string list, skip potential gaps in
ebitmap_to_strs(). All converting functions like strs_to_str(),
strs_write_each() and strs_write_each_indented() do already skip NULL
elements, but sorting such a list will lead to a NULL dereference.
#0 0x432ce5 in strcmp /src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:462:25
#1 0x4f4893 in strs_cmp selinux/libsepol/src/kernel_to_common.c:258:9
#2 0x47b74b in qsort_r /src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:9994:7
#3 0x4f481d in strs_sort selinux/libsepol/src/kernel_to_common.c:266:2
#4 0x4fe781 in attrmap_to_str selinux/libsepol/src/kernel_to_conf.c:1560:2
#5 0x4fe781 in write_type_attribute_sets_to_conf selinux/libsepol/src/kernel_to_conf.c:1599:11
#6 0x4f8098 in sepol_kernel_policydb_to_conf selinux/libsepol/src/kernel_to_conf.c:3182:7
#7 0x4e0277 in LLVMFuzzerTestOneInput selinux/libsepol/fuzz/binpolicy-fuzzer.c:50:9
#8 0x4d613b in main
#9 0x7fa2d50260b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/libc-start.c:308:16
#10 0x41d4ed in _start
Found by oss-fuzz (#44170)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The length of an ebitmap is the current highest allocated (not set) bit
and always a multiple of MAPTYPE (= 64). The role ebitmap should only
have valid role bits set, even after inverting. The length might be
smaller than the maximum number of defined roles leading to non defined
role bits set afterwards.
Only invert up to the number of roles defined instead the full ebitmap
length, similar to type_set_expand().
This also avoids timeouts on an invalid huge highbit set, since the
ebitmap has not been validated yet, on which inverting will take
excessive amount of memory and time, found by oss-fuzz (#43709).
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
For policy versions between 20 and 23 the type_val_to_struct array might
contain gaps. Skip those gaps to avoid NULL pointer dereferences:
==1250==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x00000058560b bp 0x7ffdca60c110 sp 0x7ffdca60bfc0 T0)
==1250==The signal is caused by a READ memory access.
==1250==Hint: address points to the zero page.
#0 0x58560b in build_type_map selinux/libsepol/src/optimize.c:107:33
#1 0x58560b in policydb_optimize selinux/libsepol/src/optimize.c:441:13
#2 0x55e63e in LLVMFuzzerTestOneInput selinux/libsepol/fuzz/binpolicy-fuzzer.c:42:10
#3 0x455283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) cxa_noexception.cpp:0
#4 0x440ec2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#5 0x44671c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) cxa_noexception.cpp:0
#6 0x46f522 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#7 0x7f9c160d00b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/libc-start.c:308:16
#8 0x41f67d in _start
Found by oss-fuzz (#42697)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The default log handler sepol_msg_default_handler() appends a newline
and the majority of log messages do not contain a trailing newline in
the format string.
Fixes: 5c178f9f55 ("libsepol: use logging framework in conditional.c")
Fixes: 852f14d43d ("libsepol: use logging framework in ebitmap.c")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not return success if the class name length is saturated (or too big
in the fuzzer build).
Fixes: c3d52a6a ("libsepol: check for saturated class name length")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check the from the polciy read length value to guard allocations.
In the fuzzer build the value will also be bounded to avoid oom reports.
==143646==ERROR: AddressSanitizer: allocator is out of memory trying to allocate 0xd60000000 bytes
#0 0x4dac18 in __interceptor_calloc (./out/binpolicy-fuzzer+0x4dac18)
#1 0x55d388 in scope_index_read ./libsepol/src/policydb.c:3945:7
#2 0x550097 in avrule_decl_read ./libsepol/src/policydb.c:3984:6
#3 0x550097 in avrule_block_read ./libsepol/src/policydb.c:4044:8
#4 0x54b3ac in policydb_read ./libsepol/src/policydb.c:4456:7
#5 0x518fd9 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:35:6
#6 0x43f623 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (./out/binpolicy-fuzzer+0x43f623)
#7 0x42910f in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (./out/binpolicy-fuzzer+0x42910f)
#8 0x42ee76 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (./out/binpolicy-fuzzer+0x42ee76)
#9 0x458ff2 in main (./out/binpolicy-fuzzer+0x458ff2)
#10 0x7fd37b7931c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
Found by oss-fuzz (#42909)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check the sensitivity is valid and thus the lookup in the name array
`p_sens_val_to_name` is valid.
Found by oss-fuzz (#42729, #42730, #42735, #42741)
==54784==The signal is caused by a READ memory access.
#0 0x5a10f3 in mls_semantic_level_expand ./selinux/libsepol/src/expand.c:934:11
#1 0x53839e in policydb_user_cache ./selinux/libsepol/src/policydb.c:972:7
#2 0x5c6325 in hashtab_map ./selinux/libsepol/src/hashtab.c:236:10
#3 0x5392e9 in policydb_index_others ./selinux/libsepol/src/policydb.c:1274:6
#4 0x53f90a in policydb_read ./selinux/libsepol/src/policydb.c:4496:6
#5 0x50c679 in LLVMFuzzerTestOneInput ./selinux/libsepol/fuzz/binpolicy-fuzzer.c:35:6
#6 0x4409e3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (./selinux/out/binpolicy-fuzzer+0x4409e3)
#7 0x4295bf in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (./selinux/out/binpolicy-fuzzer+0x4295bf)
#8 0x42f850 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (./selinux/out/binpolicy-fuzzer+0x42f850)
#9 0x45b6d2 in main (./selinux/out/binpolicy-fuzzer+0x45b6d2)
#10 0x7f059fcd71c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#11 0x7f059fcd7277 in __libc_start_main csu/../csu/libc-start.c:409:3
#12 0x423900 in _start (./out/binpolicy-fuzzer+0x423900)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not continue with a negative return value once a string append
operation fails to avoid increasing the buffer length variable
`str_len`, potentially leading to an out-of-bounds write.
Found by GitHub CodeQL.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check the class default targets are valid values, e.g. source or target
for user, role and type.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check all categories have valid values, especially important for
aliases.
==7888==ERROR: AddressSanitizer: SEGV on unknown address 0x602000400710 (pc 0x00000055debc bp 0x7ffe0ff2a9d0 sp 0x7ffe0ff2a8e0 T0)
==7888==The signal is caused by a READ memory access.
#0 0x55debc in write_category_rules_to_conf ./libsepol/src/kernel_to_conf.c:946:9
#1 0x55debc in write_mls_rules_to_conf ./libsepol/src/kernel_to_conf.c:1137:7
#2 0x55adb1 in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3106:7
#3 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:37:9
#4 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#5 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#6 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#7 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#8 0x7fe80ccaf7ec in __libc_start_main csu/../csu/libc-start.c:332:16
#9 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check the literal contexts in ocontext statements are defined.
==91274==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f60b0afe8c6 bp 0x7ffd42edc990 sp 0x7ffd42edc148 T0)
==91274==The signal is caused by a READ memory access.
==91274==Hint: address points to the zero page.
#0 0x7f60b0afe8c6 string/../sysdeps/x86_64/multiarch/../strlen.S:120
#1 0x4bd128 in __interceptor_strlen (./out/binpolicy-fuzzer+0x4bd128)
#2 0x5eb387 in create_str_helper ./libsepol/src/kernel_to_common.c:69:10
#3 0x5eb11e in create_str ./libsepol/src/kernel_to_common.c:99:8
#4 0x56ad7b in context_to_str ./libsepol/src/kernel_to_conf.c:2408:9
#5 0x56a717 in write_sid_context_rules_to_conf ./libsepol/src/kernel_to_conf.c:2441:9
#6 0x55b26c in write_selinux_isid_rules_to_conf ./libsepol/src/kernel_to_conf.c:2476:9
#7 0x55b26c in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3206:8
#8 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9
#9 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#10 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#11 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#12 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#13 0x7f60b0a887ec in __libc_start_main csu/../csu/libc-start.c:332:16
#14 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
==80903==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000005c0 at pc 0x0000005696c8 bp 0x7ffdb11ea560 sp 0x7ffdb11ea558
READ of size 8 at 0x6020000005c0 thread T0
#0 0x5696c7 in avtab_node_to_str ./libsepol/src/kernel_to_conf.c:1736:9
#1 0x569013 in map_avtab_write_helper ./libsepol/src/kernel_to_conf.c:1767:10
#2 0x5ab837 in avtab_map ./libsepol/src/avtab.c:347:10
#3 0x561f9a in write_avtab_flavor_to_conf ./libsepol/src/kernel_to_conf.c:1798:7
#4 0x561f9a in write_avtab_to_conf ./libsepol/src/kernel_to_conf.c:1819:8
#5 0x55afba in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3159:7
#6 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9
#7 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#8 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#9 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#10 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#11 0x7f97a83fd7ec in __libc_start_main csu/../csu/libc-start.c:332:16
#12 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check a common class or a class together with its common class parent
does not have more than the supported 32 permissions.
==28413==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f74ec3341a3 bp 0x7ffd0b7e5030 sp 0x7ffd0b7e47e8 T0)
==28413==The signal is caused by a READ memory access.
==28413==Hint: address points to the zero page.
#0 0x7f74ec3341a3 string/../sysdeps/x86_64/multiarch/../strchr.S:32
#1 0x4bfc78 in strchr (./out/binpolicy-fuzzer+0x4bfc78)
#2 0x55b7f2 in class_constraint_rules_to_strs ./libsepol/src/kernel_to_conf.c:288:7
#3 0x55b7f2 in constraint_rules_to_strs ./libsepol/src/kernel_to_conf.c:364:9
#4 0x55ac80 in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3071:7
#5 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9
#6 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#7 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#8 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#9 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#10 0x7f74ec2be7ec in __libc_start_main csu/../csu/libc-start.c:332:16
#11 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Check those contains valid values.
==57532==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000001178 at pc 0x000000564c04 bp 0x7ffed7a5ad90 sp 0x7ffed7a5ad88
READ of size 8 at 0x603000001178 thread T0
#0 0x564c03 in level_to_str ./libsepol/src/kernel_to_conf.c:1901:19
#1 0x564c03 in range_to_str ./libsepol/src/kernel_to_conf.c:1926:9
#2 0x564c03 in write_user_decl_rules_to_conf ./libsepol/src/kernel_to_conf.c:2367:12
#3 0x55b137 in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3184:7
#4 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9
#5 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#6 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#7 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#8 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#9 0x7f2c2e1a77ec in __libc_start_main csu/../csu/libc-start.c:332:16
#10 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Validate the level map of the policy to ensure no level refers to a non
existent category.
READ of size 8 at 0x602000000c58 thread T0
#0 0x568d2c in cats_ebitmap_len ./libsepol/src/kernel_to_conf.c:1003:14
#1 0x568d2c in cats_ebitmap_to_str ./libsepol/src/kernel_to_conf.c:1038:19
#2 0x55e371 in write_level_rules_to_conf ./libsepol/src/kernel_to_conf.c:1106:11
#3 0x55e371 in write_mls_rules_to_conf ./libsepol/src/kernel_to_conf.c:1140:7
#4 0x55adb1 in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3103:7
#5 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9
#6 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#7 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#8 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#9 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#10 0x7f741d0d67ec in __libc_start_main csu/../csu/libc-start.c:332:16
#11 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Split the validation of array datums regarding their gaps and entries to
simplify further checking of common classes, booleans, levels and
categories.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Currently is it implementation defined, due to the size being passed to
calloc(3), whether the operations fails nor not.
Also strs_add() does not handle a size of zero, cause it just multiplies
the size by two.
Use a default size of 1 if 0 is passed and swap the calloc arguments for
consistency.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Use the number of categories not levels, which might be zero, for the
string list initial size of categories.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Handle gaps in the user table while printing a policy configuration.
==24424==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000004bdc55 bp 0x7ffc8790b810 sp 0x7ffc8790afb0 T0)
==24424==The signal is caused by a READ memory access.
==24424==Hint: address points to the zero page.
#0 0x4bdc55 in __interceptor_strcmp (./out/binpolicy-fuzzer+0x4bdc55)
#1 0x5ebdf6 in strs_cmp ./libsepol/src/kernel_to_common.c:253:9
#2 0x505669 in __interceptor_qsort (./out/binpolicy-fuzzer+0x505669)
#3 0x5ebd84 in strs_sort ./libsepol/src/kernel_to_common.c:261:2
#4 0x564550 in write_user_decl_rules_to_conf ./libsepol/src/kernel_to_conf.c:2333:2
#5 0x55b137 in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3190:7
#6 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9
#7 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#8 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#9 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#10 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#11 0x7f530128d7ec in __libc_start_main csu/../csu/libc-start.c:332:16
#12 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Handle gaps in the class table while printing a policy configuration.
==21763==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000028 (pc 0x00000055b696 bp 0x7ffe69e8ab50 sp 0x7ffe69e8aa60 T0)
==21763==The signal is caused by a READ memory access.
==21763==Hint: address points to the zero page.
#0 0x55b696 in constraint_rules_to_strs ./libsepol/src/kernel_to_conf.c:361:14
#1 0x55ac80 in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3063:7
#2 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9
#3 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#4 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#5 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#6 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#7 0x7fc60d39e7ec in __libc_start_main csu/../csu/libc-start.c:332:16
#8 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Handle format arguments that do not have a size of at least 2.
kernel_to_common.c:69:20: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type 'unsigned long'
#0 0x557b0b in create_str_helper ./libsepol/src/kernel_to_common.c:69:20
#1 0x5577b8 in create_str ./libsepol/src/kernel_to_common.c:99:8
#2 0x56448c in cond_expr_to_str ./libsepol/src/kernel_to_conf.c:82:15
#3 0x56448c in write_cond_nodes_to_conf ./libsepol/src/kernel_to_conf.c:2103:10
#4 0x55bd9b in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3171:7
#5 0x4f9d79 in main ./checkpolicy/checkpolicy.c:684:11
#6 0x7fe2a342b7ec in __libc_start_main csu/../csu/libc-start.c:332:16
#7 0x41f3a9 in _start (./checkpolicy/checkpolicy+0x41f3a9)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The `next` member might be checked against NULL and dereferenced before
it gets assigned, due to jumps from failure gotos to the cleanup
section.
==31017==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x000000579654 bp 0x7ffd3a07d110 sp 0x7ffd3a07d000 T0)
==31017==The signal is caused by a READ memory access.
==31017==Hint: this fault was caused by a dereference of a high value address (see register values below). Disassemble the provided pc to learn which register was used.
#0 0x579654 in filename_trans_read_one ./libsepol/src/policydb.c:2874:55
#1 0x579654 in filename_trans_read ./libsepol/src/policydb.c:2902:9
#2 0x5771b7 in policydb_read ./libsepol/src/policydb.c:4509:7
#3 0x55a1f5 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:24:6
#4 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#5 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#6 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#7 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#8 0x7f2a4e7f97ec in __libc_start_main csu/../csu/libc-start.c:332:16
#9 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Free the local access vector list on failure as it does not get moved
into the policy structure.
Drop the now redundant, but non-exhaustive, resource cleanup in
cond_insertf().
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x52596d in malloc (./out/binpolicy-fuzzer+0x52596d)
#1 0x5b30d2 in cond_insertf ./libsepol/src/conditional.c:682:9
#2 0x5ac218 in avtab_read_item ./libsepol/src/avtab.c:583:10
#3 0x5b21f4 in cond_read_av_list ./libsepol/src/conditional.c:725:8
#4 0x5b21f4 in cond_read_node ./libsepol/src/conditional.c:798:7
#5 0x5b21f4 in cond_read_list ./libsepol/src/conditional.c:847:7
#6 0x576b6e in policydb_read ./libsepol/src/policydb.c:4436:8
#7 0x55a1fe in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:24:6
#8 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#9 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#10 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#11 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#12 0x7f47abeb87ec in __libc_start_main csu/../csu/libc-start.c:332:16
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Add checks for invalid read sizes from a binary policy to guard
allocations.
The common and class permission counts needs to be limited more strict
otherwise a too high count of common or class permissions can lead to
permission values with a too high value, which can lead to overflows
in shift operations.
In the fuzzer build the value will also be bounded to avoid oom reports.
==29857== ERROR: libFuzzer: out-of-memory (malloc(17179868160))
To change the out-of-memory limit use -rss_limit_mb=<N>
#0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61)
#1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o
#2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o
#3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o
#4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557)
#5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7)
#6 0x4aa143 in __asan::asan_malloc(unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa143)
#7 0x5259cb in malloc (./out/binpolicy-fuzzer+0x5259cb)
#8 0x580b5d in mallocarray ./libsepol/src/./private.h:93:9
#9 0x57c2ed in scope_read ./libsepol/src/policydb.c:4120:7
#10 0x576b0d in policydb_read ./libsepol/src/policydb.c:4462:9
#11 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6
#12 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#13 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#14 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#15 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#16 0x7ffad6e107ec in __libc_start_main csu/../csu/libc-start.c:332:16
#17 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
==19462== ERROR: libFuzzer: out-of-memory (malloc(18253611008))
To change the out-of-memory limit use -rss_limit_mb=<N>
#0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61)
#1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o
#2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o
#3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o
#4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557)
#5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7)
#6 0x4aa999 in __asan::asan_calloc(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa999)
#7 0x525b63 in __interceptor_calloc (./out/binpolicy-fuzzer+0x525b63)
#8 0x570938 in policydb_index_others ./libsepol/src/policydb.c:1245:6
#9 0x5771f3 in policydb_read ./src/policydb.c:4481:6
#10 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6
#11 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#12 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#13 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#14 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#15 0x7f4d933157ec in __libc_start_main csu/../csu/libc-start.c:332:16
#16 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Use a wrapper to guard `realloc(p, a * b)` type allocations, to detect
multiplication overflows, which result in too few memory being
allocated.
Use a custom implementation if the used C library does not offer one.
Also use temporary variables for realloc(3) results in add_i_to_a() and
fp_to_buffer().
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Use a wrapper to guard `malloc(a * b)` type allocations, to detect
multiplication overflows, which result in too few memory being
allocated.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Use the internal logging framework instead of directly writing to
stdout as it might be undesired to do so within a library.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Use the internal logging framework instead of directly writing to
stdout as it might be undesired to do so within a library.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Limit the maximum length of read sizes, like string length of module
version and name or keys and number of symtab entries. This avoids the
fuzzer to report oom events for huge allocations (it also improves the
number of executions per seconds of the fuzzer).
This change only affects the fuzzer build.
==15211== ERROR: libFuzzer: out-of-memory (malloc(3115956666))
To change the out-of-memory limit use -rss_limit_mb=<N>
#0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61)
#1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o
#2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o
#3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o
#4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557)
#5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7)
#6 0x4aa143 in __asan::asan_malloc(unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa143)
#7 0x5259cb in malloc (./out/binpolicy-fuzzer+0x5259cb)
#8 0x59d307 in str_read ./libsepol/src/services.c:1746:8
#9 0x585b97 in perm_read ./libsepol/src/policydb.c:2063:5
#10 0x581f8a in common_read ./libsepol/src/policydb.c:2119:7
#11 0x576681 in policydb_read ./libsepol/src/policydb.c:4417:8
#12 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6
#13 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#14 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#15 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#16 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#17 0x7fe1ec88a7ec in __libc_start_main csu/../csu/libc-start.c:332:16
#18 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
==12683== ERROR: libFuzzer: out-of-memory (malloc(2526451450))
To change the out-of-memory limit use -rss_limit_mb=<N>
#0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61)
#1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o
#2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o
#3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o
#4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557)
#5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7)
#6 0x4aa143 in __asan::asan_malloc(unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa143)
#7 0x5259cb in malloc (./out/binpolicy-fuzzer+0x5259cb)
#8 0x575f8a in policydb_read ./libsepol/src/policydb.c:4356:18
#9 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6
#10 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
#11 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
#12 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
#13 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
#14 0x7fa737b377ec in __libc_start_main csu/../csu/libc-start.c:332:16
#15 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Introduce a libfuzz[1] based fuzzer testing the parsing of a binary
policy.
Build the fuzzer in the oss-fuzz script.
[1]: https://llvm.org/docs/LibFuzzer.html
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not output CIL log messages while fuzzing, since their amount are
huge, e.g. for neverallow or typebounds violations.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Accept IPv4 addresses embedded in IPv6, like `::ffff:127.0.0.1`.
This allows using those in nodecon statements leading to fine grained
access control:
type=AVC msg=audit(11/29/21 20:27:44.437:419) : avc: granted { node_bind } for pid=27500 comm=intercept saddr=::ffff:127.0.0.1 src=46293 scontext=xuser_u:xuser_r:xuser_t:s0 tcontext=system_u:object_r:lo_node_t:s0 tclass=tcp_socket
This does effect policies in the traditional language due to CIL usage
in semodule(8).
Also print on conversion failures the address in question.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
With an optional file type being added to CIL genfscon rules, it
should be used when writing out a kernel policy or module to CIL
when a genfscon rule should only apply to a single security class.
Signed-off-by: James Carter <jwcart2@gmail.com>
The optional specification of a file type for a genfscon rule to
make it apply only to a specific security class is allowed by
checkpolicy and checkmodule and should be allowed for CIL policies
as well.
Allow an optional file type to be specified for a genfscon rule.
The new syntax:
(genfscon FSNAME PATH [FILE_TYPE] CONTEXT)
FSNAME - The name of the supported filesystem
PATH - If FSNAME is proc then this is the partial path,
othewise this must be "/".
FILE_TYPE - A single keyword representing the file type.
file type security class
any Same as not specifying a file type
file file
dir dir
char chr_file
block blk_file
socket sock_file
pipe fifo_file
symlink lnk_file
CONTEXT - Either a previously declared security context identifier
or an anonymous security context.
Signed-off-by: James Carter <jwcart2@gmail.com>
Prepare for the addition of an optional file type in genfscon rules
by refactoring filecon file type handling.
Make the "any" file type be the first value in enum cil_filecon_types
because it will be the most common file type.
Signed-off-by: James Carter <jwcart2@gmail.com>
Although rarely used, genfscon rules support the specification of a
file type just like the rules in a file context file. The file type
is used to make the genfscon rule apply only for a specific security
class. Currently, when writing out a policy.conf file from a kernel
policy, it is assumed that every genfscon rule applies to all security
classes and no file type will be added to the genfscon rule.
Write out the appropriate file type if the genfscon rule is only for
a specific security class (file, dir, blk_file, chr_file, fifo_file,
lnk_file, or sock_file).
Signed-off-by: James Carter <jwcart2@gmail.com>
Use string literals as format strings so that compilers can validate the
count and types of the inherent arguments.
kernel_to_cil.c: In function ‘class_constraint_rules_to_strs’:
kernel_to_cil.c:301:17: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
301 | rc = strs_create_and_add(strs, format_str, 3, classkey, perms+1, expr);
| ^~
kernel_to_cil.c: In function ‘class_validatetrans_rules_to_strs’:
kernel_to_cil.c:341:17: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
341 | rc = strs_create_and_add(strs, format_str, 2, classkey, expr);
| ^~
kernel_to_cil.c: In function ‘cats_ebitmap_to_str’:
kernel_to_cil.c:1068:40: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
1068 | val_to_name[start], val_to_name[i]);
| ^~~~~~~~~~~
kernel_to_conf.c: In function ‘class_constraint_rules_to_strs’:
kernel_to_conf.c:301:42: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
301 | flavor, classkey, perms+1, expr);
| ^~~~~~
kernel_to_conf.c: In function ‘cats_ebitmap_to_str’:
kernel_to_conf.c:1059:40: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
1059 | val_to_name[start], sep, val_to_name[i]);
| ^~~~~~~~~~~
kernel_to_conf.c:1062:25: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
1062 | len = snprintf(p, remaining, fmt, val_to_name[start]);
| ^~~
module_to_cil.c: In function ‘cond_expr_to_cil’:
module_to_cil.c:1340:25: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
1340 | rlen = snprintf(new_val, len, fmt_str, op, val1, val2);
| ^~~~
module_to_cil.c: In function ‘constraint_expr_to_string’:
module_to_cil.c:1881:25: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
1881 | rlen = snprintf(new_val, len, fmt_str, op, val1, val2);
| ^~~~
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
memcpy(3) might be annotated with the function attribute nonnull and
UBSan then complains:
module.c:296:3: runtime error: null pointer passed as argument 2, which is declared to never be null
#0 0x7f2468efa5b3 in link_netfilter_contexts ./libsepol/src/module.c:296
#1 0x7f2468efa5b3 in sepol_link_packages ./libsepol/src/module.c:337
#2 0x562331e9e123 in main ./semodule-utils/semodule_link/semodule_link.c:145
#3 0x7f2467e247ec in __libc_start_main ../csu/libc-start.c:332
#4 0x562331e9d2a9 in _start (./destdir/usr/bin/semodule_link+0x32a9)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
For the first iteration `mod->perm_map[sclassi]` is NULL, thus do not
use it as source of a memcpy(3), even with a size of 0. memcpy(3) might
be annotated with the function attribute nonnull and UBSan then
complains:
link.c:193:3: runtime error: null pointer passed as argument 2, which is declared to never be null
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
An expression of the form "1 << x" is undefined if x == 31 because
the "1" is an int and cannot be left shifted by 31.
Instead, use "UINT32_C(1) << x" which will be an unsigned int of
at least 32 bits.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
An expression of the form "1 << x" is undefined if x == 31 because
the "1" is an int and cannot be left shifted by 31.
Instead, use "UINT32_C(1) << x" which will be an unsigned int of
at least 32 bits.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since only tunableifs need to be resolved in a macro before the macro
is copied for each call, macros were being skipped after resolving
tunableifs. Statments not allowed to be in macros would be found during
the pass that resolved tunableifs. Unfortunately, in-statments are
resolved after tunableifs and they can be used to add statements to
macros that are not allowed.
Instead, do not skip macros until after the pass that resolves in-
statements that are to be resolved after block inheritance. This
allows blocks, blockinherits, blockabstracts, and macros that were
added by an in-statement to be found and an error reported.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Type bounds are checked when creating the CIL binary using libsepol
functions on the binary policy db. The bad rule is reported and, to
provide better error reporting, a search is made for matching rules
in the CIL policy. These matching rules as well as their parents are
written out with their locations to make it easier to find the rules
that violate the type bounds.
It is possible to craft CIL policies where there are many rules
that violate a bounds check each with many matching rules as well.
This can make the error messages very difficult to deal with. For
example, if there are 100 rules in the binary policy db that violate
a type bounds and each of these rules has 100 matches, then 10,000
matching rules along with their parents will be written out as part
of the error message.
Limit the error reporting to two rules for each type bounds violation
along with two matches for each of those rules.
This problem was found with the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Add an intermediate cast to uintptr_t to silence the clang specific
warning about casting a void pointer to an enum.
../cil/src/cil_verify.c:1749:28: error: cast to smaller integer type 'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
enum cil_flavor op = (enum cil_flavor)i->data;
^~~~~~~~~~~~~~~~~~~~~~~~
Similar to 32f8ed3d6b.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
GCC reports a NULL dereference of the return value of stack_peek(). This
function explicitly returns NULL in case of 'stack->pos == -1'.
Error out on NULL returned.
module_to_cil.c: In function ‘block_to_cil’:
module_to_cil.c:3357:55: error: potential null pointer dereference [-Werror=null-dereference]
3357 | struct list *alias_list = typealias_lists[decl->decl_id];
| ~~~~^~~~~~~~~
There are more occurrences of unconditionally dereferencing the return
value of stack_peek(), but the callers should ensure a valid stack, so
just silence this single warning.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The function hashtab_insert takes the type hashtab_datum_t (alias void*)
as third argument. Do not cast to hashtab_datum_t* alias void**. The
casts could be dropped, as explicit casting to void* is unnecessary, but
to fit the overall style of this file keep the casts.
expand.c:246:41: error: cast from 'perm_datum_t *' (aka 'struct perm_datum *') to 'hashtab_datum_t *' (aka 'void **') increases required alignment from 4 to 8 [-Werror,-Wcast-align]
ret = hashtab_insert(s->table, new_id, (hashtab_datum_t *) new_perm);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Mark pointers to nodes of const ebitmaps also const. C does not enforce
a transitive const-ness, but it clarifies the intent and improves
maintainability.
Follow-up of 390ec54d27
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
When checking for circular class permission declarations and a class
mapping is encountered, the class permissions for each map permission
must be checked. An assumption was made that there were no operators
in the class permissions. An operator in the class permissions would
cause a segfault.
Example causing segault:
(classmap cm1 (mp1))
(classmapping cm1 mp1 (CLASS (PERM)))
(classpermission cp1)
(classpermissionset cp1 (cm1 (all)))
For map class permissions, check each item in the permission list to
see if it is an operator. If it is not, then verify the class
permissions associated with the map permission. If it is an operator
and the operator is "all", then create a list of all permissions for
that map class and verify the class permissions associated with each
map permission. If it is a different operator, then it can be skipped.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
When compiling CIL policy using secilc's "-m" option (which allows
duplicate declarations for types and type attributes), a segfault
will occur if the type or type attribute being copied has already
been declared. This is because a search of the symbol table is made
during the copy and the original datum will be used if one is found.
The original datum will be considered a duplicate when an attempt is
made to add it to the symbol table. The original datum, which is still
in use, will then be destroyed and a segfault will follow soon after
that.
Instead, always create a new datum. When it is added the new datum
will be destroyed if it is a duplicate and duplicate declarations
are allowed.
Signed-off-by: James Carter <jwcart2@gmail.com>
Found while running the checkpolicy/test/dispol binary.
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x49bacd in __interceptor_malloc (./checkpolicy/test/dispol+0x49bacd)
#1 0x5551e1 in ebitmap_set_bit ./libsepol/src/ebitmap.c:326:27
#2 0x517873 in create_gap_ebitmap ./libsepol/src/policydb_validate.c:23:8
#3 0x517873 in validate_init ./libsepol/src/policydb_validate.c:34:6
#4 0x50fa47 in validate_array_init ./libsepol/src/policydb_validate.c:44:6
#5 0x50fa47 in validate_policydb ./libsepol/src/policydb_validate.c:732:6
#6 0x4f22df in policydb_read ./libsepol/src/policydb.c:4538:6
#7 0x4cddb3 in main ./checkpolicy/test/dispol.c:437:8
#8 0x7f5980e47e49 in __libc_start_main csu/../csu/libc-start.c:314:16
Indirect leak of 48 byte(s) in 2 object(s) allocated from:
#0 0x49bacd in __interceptor_malloc (./checkpolicy/test/dispol+0x49bacd)
#1 0x5551e1 in ebitmap_set_bit ./libsepol/src/ebitmap.c:326:27
#2 0x517873 in create_gap_ebitmap ./libsepol/src/policydb_validate.c:23:8
#3 0x517873 in validate_init ./libsepol/src/policydb_validate.c:34:6
#4 0x50fa47 in validate_array_init ./libsepol/src/policydb_validate.c:44:6
#5 0x50fa47 in validate_policydb ./libsepol/src/policydb_validate.c:732:6
#6 0x4f22df in policydb_read ./libsepol/src/policydb.c:4538:6
#7 0x4cddb3 in main ./checkpolicy/test/dispol.c:437:8
#8 0x7f5980e47e49 in __libc_start_main csu/../csu/libc-start.c:314:16
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.
util.c:95:15: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Duplicate declarations are allowed for type, typeattribute, and
optional statements. When an allowed duplicate declaration is found,
the duplicate datum is free'd in cil_add_decl_to_symtab() and SEPOL_OK
is returned. This works for all the rules where a duplicate declaration
is allowed, but it confuses scanning tools.
When cil_add_decl_to_symtab() finds an allowed duplicate declaration,
return SEPOL_EEXIST and free the duplicate datum in the original
calling function.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
libsepol/cil/src/cil_binary.c:4823: alloc_arg: "bounds_check_type" allocates memory that is stored into "bad".
libsepol/cil/src/cil_binary.c:4840: var_assign: Assigning: "cur" = "bad".
libsepol/cil/src/cil_binary.c:4844: noescape: Resource "cur" is not freed or pointed-to in "cil_avrule_from_sepol".
libsepol/cil/src/cil_binary.c:4847: leaked_storage: Variable "cur" going out of scope leaks the storage it points to.
libsepol/cil/src/cil_binary.c:4847: leaked_storage: Variable "bad" going out of scope leaks the storage it points to.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
A line mark functions like an open parenthesis, so the number of
active line marks should be limited like the number of open
parenthesis.
This issue was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Fixes:
Error: RESOURCE_LEAK (CWE-772): [#def5]
libsepol/src/kernel_to_cil.c:2380: alloc_arg: "strs_init" allocates memory that is stored into "strs".
libsepol/src/kernel_to_cil.c:2386: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_cil.c:2386: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_cil.c:2386: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_cil.c:2507: leaked_storage: Variable "strs" going out of scope leaks the storage it points to.
libsepol/src/kernel_to_conf.c:2315: alloc_arg: "strs_init" allocates memory that is stored into "strs".
libsepol/src/kernel_to_conf.c:2321: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_conf.c:2321: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_conf.c:2321: noescape: Resource "strs" is not freed or pointed-to in "strs_add".
libsepol/src/kernel_to_conf.c:2385: leaked_storage: Variable "strs" going out of scope leaks the storage it points to.
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: James Carter <jwcart2@gmail.com>
The function __cil_verify_syntax() is used to check the syntax of
CIL rules (and a few other common things like contexts and class
permissions). It does not correctly check the syntax combination
"CIL_SYN_STRING | CIL_SYN_N_LISTS, CIL_SYN_N_LISTS | CIL_SYN_END".
This should mean either a string followed by any number of lists
or any number of lists followed by the end of the rule. Instead,
while allowing the correct syntax, it allows any number of lists
followed by a string followed by any number of more lists followed
by the end of the rule and, also, any number of lists followed by a
string followed by the end of the rule.
Refactor the function to make it clearer to follow and so that once
checking begins for CIL_SYN_N_LISTS or CIL_SYN_N_STRINGS, then only
strings or lists are allowed until the end of the rule is found. In
addition, always check for CIL_SYN_END at the end.
Signed-off-by: James Carter <jwcart2@gmail.com>
Since the value passed into __cil_verify_syntax() as the len
parameter is always calculated from sizeof(syntax)/sizeof(*syntax),
use size_t for the calculated value in the calling function and for
the len parameter. In __cil_verify_syntax(), the variable i is only
compared to len, so make that size_t as well.
Signed-off-by: James Carter <jwcart2@gmail.com>
For every call to cil_fill_classperms_list(), the syntax of the
whole rule, including the class permissions, has already been
checked. There is no reason to check it again. Also, because the
class permissions appear in the middle of some rules, like
constraints, the syntax array does not end with CIL_SYN_END. This
is the only case where the syntax array does not end with CIL_SYN_END.
This prevents __cil_verify_syntax() from requiring that the syntax
array ends with CIL_SYN_END.
Remove the redundant syntax checking in cil_fill_classperms_list().
Signed-off-by: James Carter <jwcart2@gmail.com>
CIL's in-statement is resolved before block inheritance. This has
the advantage of allowing an in-statement to add rules to a base
block (say for a new permission) and having those rules also be
added everywhere that base block is inherited. But the disadvantage
of this behavior is that it is not possible to use an in-statement
on a block that is inherited for the simple reason that that block
does not exist when the in-statment is resolved.
Change the syntax of the in-statement to allow specifying whether
the rules should be added before or after inheritance. If neither
is specified, then the behavior remains the same. All current
in-statements will work as before.
Either the old syntax
(in container_id
cil_statement
...
)
or the new syntax
(in before|after container_id
cil_statement
...
)
may be used for in-statements. But only "(in after ..." will have
the new behavior. Using "(in before ..." will give the same
behavior as before.
Macro Example
;
(block b1
(macro m1 ((type ARG1))
(allow ARG1 self (C1 (P1a)))
)
)
(in after b1.m1
(allow ARG1 self (C1 (P1c)))
)
(type t1a)
(call b1.m1 (t1a))
(blockinherit b1)
(in after m1
(allow ARG1 self (C1 (P1b)))
)
(type t1b)
(call m1 (t1b))
;
This results in the following rules:
(allow t1a self (C1 (P1a)))
(allow t1a self (C1 (P1c)))
(allow t1b self (C1 (P1a)))
(allow t1b self (C1 (P1b)))
Block Example
;
(block b2
(block b
(type ta)
(allow ta self (C2 (P2a)))
)
)
(in before b2.b
(type tb)
(allow tb self (C2 (P2b)))
)
(block c2
(blockinherit b2)
(in after b
(type tc)
(allow tc self (C2 (P2c)))
)
)
;
This results in the following rules:
(allow b2.b.ta self (C2 (P2a)))
(allow b2.b.tb self (C2 (P2b)))
(allow c2.b.ta self (C2 (P2a)))
(allow c2.b.tb self (C2 (P2b)))
(allow c2.b.tc self (C2 (P2c)))
Using in-statements on optionals also works as expected.
One additional change is that blockabstract and blockinherit rules
are not allowed when using an after in-statement. This is because
both of those are resolved before an after in-statement would be
resolved.
Signed-off-by: James Carter <jwcart2@gmail.com>
Use a simpler recursive solution and set the head and tail pointers
of the starting node to NULL when done.
Remove the now uneeded setting of the head and tail pointers to NULL
in cil_resolve_in().
Signed-off-by: James Carter <jwcart2@gmail.com>
Refactor the function __cil_build_ast_node_helper() by moving the
check for illegal statements and the large if-then-else statement
to determine which function to call to parse the policy statements
to different functions.
There is no need to keep walking the nodes of a policy statement
that has already been completely parsed. This means that the
remaining nodes of any policy statement that does not contain a list
of policy statements can be skipped. This was done inconsistently
before. The following policy statements now have all nodes after
the first one skipped: blockinherit, blockabstract, classcommon,
user, userattribute, userbounds, userprefix, type, typeattribute,
typealias, typealiasactual, typebounds, typepermissive, role,
userrole, roletype, roletransition, roleallow, roleattribute,
rolebounds, bool, tunable, typetransition, typechange, typemember,
sensitivity, sensitivityalias, senistivityaliasactual, category,
categoryalias, categoryaliasactual, and ipaddr. The only policy
statements that do contain a list of policy statements are:
block, in, tunableif, booleanif, true (conditional block), false
(conditional block), macro, optional, and src_info.
Signed-off-by: James Carter <jwcart2@gmail.com>
If an optional that is to be disabled is the child of an optional that
is going to be disabled, then there is no reason to add that optional
to the stack of disabled optionals, because it is going to be destroyed
anyways. This means that there is no reason to maintain a stack of
disabled optionals at all.
Instead of using a stack to track disabled optionals, use a pointer
that points to the top-most optional that is to be disabled. When a
rule fails to resolve in an optional, if the disabled optional pointer
has not been set, then set it to that optional. If the pointer has
been set already, then the optional is already going to be destroyed,
so nothing else needs to be done. The resolution failure and the fact
that the optional is being disabled is reported in either case.
Signed-off-by: James Carter <jwcart2@gmail.com>
File names for typetransition rules are stored in their own datums.
This allows them to be passed as a parameter, but there needs to be
a check in __cil_insert_name() so that parameter names are not
mistaken for file name strings. This check did not verify that a
matching parameter name had the flavor of CIL_NAME.
Check that the parameter flavor is CIL_NAME and that the paramter
name matches the file name to be stored in the datum.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
A list is created to store type attribute datums when resolving an
expandtypeattribute rule and that list needs to be destroyed if the
AST is reset or a memory leak will occur.
Destroy the list storing type attributes datums when resetting
expandtypeattribute rules.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
The function cil_tree_get_next_path() does not check whether the
parse tree node that stores the high-level language file path of a
src_info rule actually exists before trying to read the path. This
can result in a NULL dereference.
Check that all of the parse tree nodes of a src_info rule exist
before reading the data from them.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
The commit d155b410d4 (libsepol/cil:
Check for duplicate blocks, optionals, and macros) added checks when
copying blocks, macros, and optionals so that a duplicate would cause
an exit with an error. Unfortunately, some policies exist that depend
on this behavior when using inheritance.
The behavior is as follows.
For macros only the first declared macro matters.
;
(macro m ((type ARG1))
(allow ARG1 self (CLASS (PERM1)))
)
(block b
(macro m ((type ARG1))
(allow ARG1 self (CLASS (PERM2)))
)
)
(blockinherit b)
(type t)
(call m (t))
;
For this policy segment, the macro m in block b will not be called.
Only the original macro m will be.
This behavior has been used to override macros that are going to
be inherited. Only the inherited macros that have not already been
declared in the destination namespace will be used.
Blocks seem to work fine even though there are two of them
;
(block b1
(blockinherit b2)
(block b
(type t1)
(allow t1 self (CLASS (PERM)))
)
)
(block b2
(block b
(type t2)
(allow t2 self (CLASS (PERM)))
)
)
(blockinherit b1)
;
In this example, the blockinherit of b2 will cause there to be
two block b's in block b1. Note that if both block b's tried to
declare the same type, then that would be an error. The blockinherit
of b1 will copy both block b's.
This behavior has been used to allow the use of in-statements for
a block that is being inherited. Since the in-statements are resolved
before block inheritance, this only works if a block with the same
name as the block to be inherited is declared in the namespace.
To support the use of these two behaviors, allow duplicate blocks
and macros when they occur as the result of block inheritance. In
any other circumstances and error for a redeclaration will be given.
Since the duplicate macro is not going to be used it is just skipped.
The duplicate block will use the datum of the original block. In both
cases a warning message will be produced (it will only be seen if
"-v" is used when compiling the policy).
Signed-off-by: James Carter <jwcart2@gmail.com>
In order to retain as much information as possible, when writing
out the CIL AST, use line mark notation to write out src_info
nodes. This includes using line marks to denote the original CIL
files the AST comes from.
The line numbers will not always be exactly correct because any
blank lines and comments in the original files will not be
represented in the AST.
Line marks are not written for the parse tree because the line
numbers will be widely inaccurate since each token will be on
a different line.
Signed-off-by: James Carter <jwcart2@gmail.com>
CIL supports specifiying the original high-level language file and
line numbers when reporting errors. This is done through line marks
and is mostly used to report the original Refpolicy file and line
number for neverallow rules that have been converted to CIL.
As long as the line mark remain simple, everything works fine, but
the wrong line numbers will be reported with more complex nextings
of line marks.
Example:
;;* lms 100 file01.hll
(type t1a)
(allow t1a self (CLASS (PERM)))
;;* lmx 200 file02.hll
(type t2a)
(allow t2a self (CLASS (PERM)))
;;* lme
(type t1b)
(allow t1b self (CLASS (PERM)))
(allow bad1b self (CLASS (PERM))) ; file01.hll:101 (Should be 106)
;;* lme
The primary problem is that the tree nodes can only store one hll
line number. Instead a number is needed that can be used by any
number of stacked line mark sections. This number would increment
line a normal line number except when in lmx sections (that have
the same line number throughout the section because they represent
an expansion of a line -- like the expansion of a macro call. This
number can go backwards when exiting a lms section within a lmx
section, because line number will increase in the lms section, but
outside the lmx section, the line number did not advance.
This number is called the hll_offset and this is the value that is
now stored in tree nodes instead of the hll line number. To calculate
the hll line number for a rule, a search is made for an ancestor of
the node that is a line mark and the line number for a lms section
is the hll line number stored in the line mark, plus the hll offset
of the rule, minus the hll offset of the line mark node, minus one.
(hll_lineno + hll_offset_rule - hll_offset_lm - 1)
Signed-off-by: James Carter <jwcart2@gmail.com>
To be able to write line mark information when writing the AST,
the line mark kind and line number is needed in the src info.
Instead of indicating whether the src info is for CIL or a hll,
differentiate between CIL, a normal hll line mark, and an expanded
hll line mark. Also include the line mark line number in the src
info nodes.
Signed-off-by: James Carter <jwcart2@gmail.com>
The functions cil_fill_integer() and cil_fill_integer64() exist in
cil_build_ast.c, but these functions take a node and it would be
better to have a function that can be used in add_hll_linemark()
so that the common functinality is in one place.
Create cil_string_to_uint32() and cil_string_to_uint64() and use
these functions in cil_fill_integer(), cil_fill_integer64(), and
add_hll_linemark().
Signed-off-by: James Carter <jwcart2@gmail.com>
CIL line mark rules are used to annotate the original line and file
of a rule. It is mostly used for neverallow rules that have been
converted to CIL.
Pushing the current line mark state after processing a line mark
section does not make sense since that information is never used.
When the line mark section ends the information is just popped and
discarded. It also makes pop_hll_info() more complicated than it
needs to be.
Push the line mark state first and simplfy pop_hll_info().
Signed-off-by: James Carter <jwcart2@gmail.com>
