Alan T. DeKok [Wed, 11 Sep 2024 13:27:57 +0000 (09:27 -0400)]
check the right parent
the outer parent_map is taken from starting to parse the section.
map_afrom_cs() will parse multiple child sections, and call the
validation function for each one. But will only pass the top-level
parent_map.
So fixup the ref to use the actual parent when there are multiple
levels
James Jones [Tue, 13 Aug 2024 21:11:41 +0000 (16:11 -0500)]
Annotate overflow issues in lo_read() (CID #1604601)
Coverity sets itself up in a vicious cycle:
1. It considers the loop check expression to be tainted because
total is tainted, so outlen is tainted, too.
2. Because of that, outlen - total (passed to read()) is deemed
overflowed, so the return value r is considered overflowed.
3. Returning total, which is considered overflowed, is another issue.
4. r, which is considered overflowed, is added to r--which is why
total considered to have overflowed and hence be tainted.
Once we changed the code to not add r to total in the EINTR case,
one can, but Coverity cannot, infer that total will only take on
values in {0,1,...,outlen}, and since both have the same type, total
can represent all such values. read(), as a standard function, is
one it should have a model for, but it doesn't seem to include the
property that the returned value is less than or equal to the passed
number of bytes to read(), and it doesn't have a way to let us
represent it in a custom model.
James Jones [Mon, 12 Aug 2024 14:34:39 +0000 (09:34 -0500)]
Annotate overflow in event_fd_func_index_build() (CID #164609)
Coverity-only check won't work, because the only one that makes
sense would be for pos == 0... but the while loop condition,
which Coverity can see, checks exactly that, leaving us with
annotation.
James Jones [Thu, 8 Aug 2024 21:15:05 +0000 (16:15 -0500)]
Add Coverity-only check to avoid false positive overflow (CID 1604621)
Coverity doesn't know at this point that fr_high_bit_pos() will
necessarily return a value between 5 and 64, so that ret will
have a value in {1, 2, ..., 8}, NOT 2305843009213693952. We add
a check only coverity will see to convince it there is no overflow.
James Jones [Thu, 8 Aug 2024 13:35:02 +0000 (08:35 -0500)]
Revise write_all() to avoid overflow (CID #1604608)
write_all() len parameter is changed to size_t so len - done is
calculated as size_t to try to avoid an over or underflow Coverity
claims occurs. For simplicity and to avoid another overflow complaint,
write_all() now returns 0 for success and -1 for error.
James Jones [Tue, 30 Jul 2024 19:32:45 +0000 (14:32 -0500)]
Add coverity-only check to _fr_dbuff_in_uint64v() (CID #1604617)
Coverity doesn't realize that the value fr_high_bit_pos() returns
is necessarily between 4 and 64, so that ret is between 1 and 8
so that sizeof(uint64_t) - ret will never underflow. We add the
test for Coverity only to pacify it.
Coverity sees the initialization of hash and the multiplication by
FNV_MAGIC_PRIME and points out that the product is too large for a
uint32_t, but because the multiplication is done in an unsigned type,
that is defined behavior and the intended behavior for the hash
functions.
James Jones [Mon, 1 Jul 2024 14:46:44 +0000 (09:46 -0500)]
Annotate Coverity false positive for the parse-only case (CID #1604604)
out is assigned NULL, and for the parse-only case stays that way. Then
call_env_result() is called, which doesn't dereference out in the
parse-only case, but Coverity doesn't realize it and hence complains.
James Jones [Tue, 3 Sep 2024 19:23:15 +0000 (14:23 -0500)]
use strlcpy()i rather than strcpy() (CIDs #1618878-#161880)
command_encode_dns_label(), command_radmin_add(), and
command_encode_raw() all strcpy() the incoming string in to a local
fixed-size buffer buffer. The callers all pass a pointer to a buffer
no bigger than the local buffer, but Coverity apparently cannot tell
that. (It looks like all calls to hem are made through an array
of structures--perhaps that's why.) To pacify Coverity, we switch
from strcpy() to strlcpy(), which takes an extra parameter to let
us guarantee it won't overrun buffer.
Alan T. DeKok [Tue, 10 Sep 2024 12:12:16 +0000 (08:12 -0400)]
don't cast xlat function arguments
if the output of the xlat is supposed to be cast, that shouldn't
affect the inputs to the xlat arguments.
In fact, we should probably update xlat_tokenize_argv() to cast
each argument as it's parsing, which should serve as a hint to
the parser as to what to expect.