From: James Jones <jejones3141@gmail.com>
Date: Mon, 29 Aug 2022 19:39:07 +0000 (-0500)
Subject: 	Deal with (possibly false positive) untrusted values. (CIDs below) (#4701)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f77f8158b350a8ef007b6a62c70aaf028e8a55e6;p=thirdparty%2Ffreeradius-server.git

	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 5d8760d7409..248afffbd11 100644
--- 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 f301737a63b..b7719c57fd5 100644
--- 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 6a21ccf5e92..3e1dab25407 100644
--- 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 8432cad389a..440dad8ed93 100644
--- 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 ac48929fd9a..53389a8cc5c 100644
--- 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 770bd7fa402..c557a253e6c 100644
--- 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);
 	}
 }