Deal with (possibly false positive) untrusted values. (CIDs below) (#4701)

1445221: length is checked for consistency with packet length;
         annotated.
1448175: pulled the checksum validation into the block declaring
         udp_len, to make it clear that it *is* range checked.
         Annotated anyway, because coverity hasn't noticed other
         such clear range checks.
1448175: len is range checked just before the call to memcpy(),
         which is what coverity says should be done. Annotated.
1503937: the check of p effectively sanity checks count; annotated.
1503954: before the fr_pair_tlvs_from_network() call, option_len
         is indeed checked; annotated.
1503968: length is checked; perhaps coverity doesn't recognize
         that (option + 4 + length) > end is equivalent to
         length > end - (option + 4). Annotated.

Co-authored-by: Alan DeKok <aland@freeradius.org>

diff --git a/src/bin/radsniff.c b/src/bin/radsniff.c
index 5d8760d7409d5a7ac8ac1e20098694181dd0f365..248afffbd11226d6245df9e135e225e1cdf8efa8 100644 (file)
--- a/src/bin/radsniff.c
+++ b/src/bin/radsniff.c
@@ -1370,16 +1370,17 @@ static void rs_packet_process(uint64_t count, rs_event_t *event, struct pcap_pkt
                        return;
                }
 #endif
-       }
-       if ((version == 4) && conf->verify_udp_checksum) {
-               uint16_t expected;
-
-               expected = fr_udp_checksum((uint8_t const *) udp, ntohs(udp->len), udp->checksum,
-                                          ip->ip_src, ip->ip_dst);
-               if (udp->checksum != expected) {
-                       REDEBUG("UDP checksum invalid, packet: 0x%04hx calculated: 0x%04hx",
-                               ntohs(udp->checksum), ntohs(expected));
+               if ((version == 4) && conf->verify_udp_checksum) {
+                       uint16_t expected;
+
+                       /* coverity[tainted_data] */
+                       expected = fr_udp_checksum((uint8_t const *) udp, udp_len, udp->checksum,
+                                                  ip->ip_src, ip->ip_dst);
+                       if (udp->checksum != expected) {
+                               REDEBUG("UDP checksum invalid, packet: 0x%04hx calculated: 0x%04hx",
+                                       ntohs(udp->checksum), ntohs(expected));
                        /* Not a fatal error */
+                       }
                }
        }
        p += sizeof(udp_header_t);

diff --git a/src/listen/dhcpv6/proto_dhcpv6.c b/src/listen/dhcpv6/proto_dhcpv6.c
index f301737a63bf1a40e1de009fb5d85d7c4e558a30..b7719c57fd50b78abdbf9bc794ecc2beb1938b4f 100644 (file)
--- a/src/listen/dhcpv6/proto_dhcpv6.c
+++ b/src/listen/dhcpv6/proto_dhcpv6.c
@@ -332,8 +332,9 @@ static ssize_t mod_encode(void const *instance, request_t *request, uint8_t *buf
 
                client_id = fr_dhcpv6_option_find(request->packet->data + 4, request->packet->data + request->packet->data_len, attr_client_id->attr);
                if (client_id) {
-                       size_t len = (client_id[2] << 8) | client_id[3];
+                       size_t len = fr_nbo_to_uint16(client_id + 2);
                        if ((data_len + 4 + len) <= buffer_len) {
+                               /* coverity[tainted_data} */
                                memcpy(buffer + data_len, client_id, 4 + len);
                                data_len += 4 + len;
                        }

diff --git a/src/protocols/dhcpv4/decode.c b/src/protocols/dhcpv4/decode.c
index 6a21ccf5e92c5784808a537fa51d92f63dd60ee6..3e1dab2540710cb5cb4a14c46ccf9041c633da9a 100644 (file)
--- a/src/protocols/dhcpv4/decode.c
+++ b/src/protocols/dhcpv4/decode.c
@@ -403,6 +403,7 @@ next:
        }
        p++;
 
+       /* coverity[tainted_data] */
        len = fr_pair_tlvs_from_network(ctx, out, vendor, p, option_len, decode_ctx, decode_option, verify_tlvs, false);
        if (len <= 0) return len;
 

diff --git a/src/protocols/dhcpv6/base.c b/src/protocols/dhcpv6/base.c
index 8432cad389abf8e4cd610f10758db8e08ac58717..440dad8ed93e7f2279ada1a38c60a5209d71e085 100644 (file)
--- a/src/protocols/dhcpv6/base.c
+++ b/src/protocols/dhcpv6/base.c
@@ -886,6 +886,7 @@ static void dhcpv6_print_hex(FILE *fp, uint8_t const *packet, size_t packet_len,
                        break;
                }
 
+               /* coverity[tainted_data] */
                print_hex_data(fp, option + 4, length, depth + 3);
                if ((option[0] == 0) && (option[1] == attr_relay_message->attr)) {
                        dhcpv6_print_hex(fp, option + 4, length, depth + 2);

diff --git a/src/protocols/dns/decode.c b/src/protocols/dns/decode.c
index ac48929fd9a1685616cee77120cf49e6bbfbdf9f..53389a8cc5ca61c968a22f16b7fc054e8dd9bfbd 100644 (file)
--- a/src/protocols/dns/decode.c
+++ b/src/protocols/dns/decode.c
@@ -236,6 +236,7 @@ static ssize_t decode_record(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_
 
        count = fr_nbo_to_uint16(counter);
        FR_PROTO_TRACE("Decoding %u of %s", count, attr->name);
+       /* coverity[tainted_data] */
        for (i = 0; i < count; i++) {
                ssize_t slen;
 

diff --git a/src/protocols/vmps/vmps.c b/src/protocols/vmps/vmps.c
index 770bd7fa4022560a07f97021745bc178c687ef4e..c557a253e6cba5d16269c1335e2d8bc8ec5ceda5 100644 (file)
--- a/src/protocols/vmps/vmps.c
+++ b/src/protocols/vmps/vmps.c
@@ -494,6 +494,7 @@ void fr_vmps_print_hex(FILE *fp, uint8_t const *packet, size_t packet_len)
 
                fprintf(fp, "\t\t%08x  %04x  ", id, length);
 
+               /* coverity[tainted_data] */
                print_hex_data(attr + 6, length - 6, 3);
        }
 }