Alan T. DeKok [Thu, 22 Sep 2022 18:46:00 +0000 (14:46 -0400)]
clean up integer calculations
simplify the process of hinting, and always upcast intermediate
integers to int64_t or uint64_t. That way the result is much more
likely to be representable
James Jones [Fri, 23 Sep 2022 11:45:02 +0000 (06:45 -0500)]
initialize buff to placate coverity (CIDs #1503942, #1504042) (#4738)
Coverity doesn't realize that fr_sbuff_out_bstrncpy_allowed()
will put something in buff, if only a NUL terminator. Until
coverity sees annotations in macro definitions, the only way
I know of to avoid the false positive "uninit_use_in_call"
defects in SBUFF_PARSE_FLOAT_DEF()-generated functions is to
actually initialize buff.
James Jones [Wed, 21 Sep 2022 13:53:39 +0000 (08:53 -0500)]
Annotate calls we know won't fail (CID #1503939) (#4722)
The individual tests in edit_tests all operate on a three-element
pair list they get from add_pairs(), freeing it after they're done.
fr_pair_afrom_da() only returns NULL if it runs out of memory--
unlikely in this context, so we annotate the calls that add items
to the list.
James Jones [Wed, 21 Sep 2022 13:52:43 +0000 (08:52 -0500)]
Check inst->filename before trying to use it (CID #1503909) (#4732)
It appears that all invocations of the open function of an
fr_app_io_t check for error, so this should suffice (apart
from possible error loggign) and should satisfy coverity
without annotation.
James Jones [Wed, 21 Sep 2022 13:51:58 +0000 (08:51 -0500)]
Annotate "tainted data" in fr_udp_header_check() call (CID #1504049) (#4727)
Coverity claims that the "downcast" of a uint8_t const * to
ip_header_t const * taints "the data that this pointer points to".
"this pointer" must be p, because coverity says not a word about
dereferences of ip. The pointed at data isn't changed, being const
qualified. p and the passed length are checked in fr_udp_header_check().
Alan T. DeKok [Tue, 20 Sep 2022 16:51:38 +0000 (12:51 -0400)]
reset request->module only when the module is done
not when it returns. Because if the module asks for an xlat to
be expanded, that is done on behalf of the module. And any xlat
debugging should print the module name
Alan T. DeKok [Sat, 17 Sep 2022 13:27:18 +0000 (09:27 -0400)]
save node->fmt as the name of the attribute we're looking for
we should arguably walk through the xlat after it's been parsed,
and replace node->fmt with the *printed* version of the node.
That way we don't have to resort to run-time walking through the
func+argc, etc. in order to print a name for the xlat we're
resolving.
find is uninitialized and only has a small portion of find.addr.inet
set in fr_redis_cluster_pool_by_node_addr(), but then all of find.addr
is assigned to spare->pending_addr, hence coverity complains. It turna
out, though, that cluster_node_connect() only uses the part that did
get set.
In fr_dhcv4_raw_packet_recv() (is there a reason for that
spelling?), coverity claims the downcast of packet->data
in the fr_dhcpv4_packet_get_option() call taints the contents
of packet->data, but it's cast to, and the called function
takes, a const-qualified pointer, so in what sense can it be
tainted?
James Jones [Mon, 12 Sep 2022 23:34:17 +0000 (18:34 -0500)]
Deal with coverity tainted data defect in mod_track_create() (CID #14… (#4718)
* Deal with coverity tainted data defect in mod_track_create() (CID #1469134)
Added a test to make sure option + option_len doesn't extend off
the end of the packet. Since in other cases, coverity doesn't
recognize the range check that it asks for, we annotate it, too.
James Jones [Mon, 12 Sep 2022 23:30:52 +0000 (18:30 -0500)]
Check for NULL return from xlat_register() (CID #1507059) (#4721)
Barring issues with args, xlat_func_args() will dereference the
xlat_t * handed it, and xlat_register()'s callers do check for
error return, so it makes sense to check what xlat_register()
returns and return error in that case.
1503938: flags is large enough that fr_dict_attr_flags_print()
will not run out of space, the only way it can return
an error. 1504037: NULL is passed as the ancestor, and coverity doesn't
complain about evaluating da->type, so it must not
think da is NULL. Given that, the fr_dict_attr_oid_print()
call here also can only fail by running out of room.
oid_str is 512 bytes, but here the length depends on
da->depth. It appears that 512 bytes suffices in practice,
but we'll check the return value and exit after logging
an error if it proves not to.